fix(quickfiler): repair three keyboard-action contract defects (#445)

[drmoisan]

Summary

Repairs three contract defects in the QuickFiler keyboard-action family. Closes#445.

This is a wave-0 child of the quickfiler-suite-determinism-foundation epic and targets the epic integration branch, not main.

Defect 1 — inconsistent Activated gating in KaStringAsync.KeyEquals

Branches 1 and 2 gated their side effects on Activated; branch 3 (other.Length > 1) invoked Update without the gate. A non-matching multi-character probe therefore fired its side effect on every LINQ re-enumeration within a single keystroke. All three branches now gate on Activated && Update is not null.

Branch 1's early return is deliberately preserved rather than "completed" into a fall-through to the trailing latch reset. KeyboardHandler re-arms the latch only at filter length 1 and then makes three passes per keystroke; if a matching probe cleared the latch, the first pass would consume the activation and the item-number label would stop advancing. A new test pins that behaviour, and the rationale is recorded in the method's XML documentation.

Defect 2 — KeyEquals("") had no defined contract

Key.Contains("") is true for every receiver, so an empty probe entered branch 1 and, with Activated true and a non-null Update, evaluated Key.Substring(other.Length - 1, 1) with a start index of -1 and threw ArgumentOutOfRangeException. With the guard false it silently returned true, so an empty probe matched every registered action.

KeyEquals now rejects null with ArgumentNullException and empty with ArgumentException, from a guard clause placed above the Key.Contains(other) test. The negative start index is unreachable.

Defect 3 — KaChar.DelegateType reported a type it does not store

KaChar stores an Action<char> but DelegateType returned typeof(Action<Keys>). Because the corresponding interface member was commented out, DelegateType was orphaned public API on KaChar and KaKey with no caller, so it is removed rather than corrected. A repository-wide search over *.cs now returns zero hits.

The dead Update property and its backing field are removed from KaChar, KaCharAsync, KaKey, and KaKeyAsync, and retained on KaStringAsync, which reads it. Both commented-out members are deleted from IKbdAction; its four live members are byte-identical, so no implementer signature moves.

Behaviour change and blast radius

The empty-probe precondition is a real behaviour change: the KbdActions string-keyed members (ContainsKey, FilterKeys, Find, FindIndex, and the indexer) inherit it, and an empty key argument now surfaces an ArgumentException rather than matching everything.

Review traced every live caller and found no production path that reaches the new throw:

• Keyboard filter probes are always length 1 or greater, because the append at KeyboardHandler.cs:180 precedes the probes at :181 and :188.
• Indexer probes use registered keys, which are never empty (digits is 1 or 2 on every path).
• Remove and Add route through StoredKeyEquals, not KeyEquals.

The consequence is documented in the KeyEquals XML documentation so a future caller is warned at the API surface.

Out of scope, and filed

Branch 1 guards on Key.Contains(other) but computes Key.Substring(other.Length - 1, 1), an offset that is only meaningful when other is a prefix of Key. This is a fourth, distinct defect whose resolution requires choosing between Contains and StartsWith — a keyboard-filtering behaviour change currently pinned by an existing test. It is deliberately not fixed here and is filed as #583.

Verification

No GitHub Actions workflow runs on this pull request: .github/workflows/ci.yml triggers pull_request only on [main, development], and this PR targets the epic integration branch. The absence of checks is expected and is not a failure. The green gate is the full local C# toolchain, run in policy order to a single uninterrupted clean pass.

Stage	Result
csharpier check .	1517 files checked, 0 needing formatting
msbuild analyzers (/t:Rebuild)	exit 0, 0 errors, 5 pre-existing third-party advisories
msbuild nullable (/t:Rebuild)	exit 0, 0 errors, 5 pre-existing third-party advisories
vstest (/InIsolation, coverage)	6441 passed, 0 failed, 0 skipped across 9 assemblies

Both MSBuild logs record a Skipping target "CoreCompile" count of 0 (and CoreCompile: counts of 100 and 111), which proves the analyzer and nullable gates actually compiled rather than short-circuiting on an incremental up-to-date check. The test baseline before the change was 6437 passed / 0 failed; the delta of exactly +4 is the four new tests.

Coverage

	Before	After
Line rate	70.5971%	70.6037%
Branch rate	58.7406%	58.7469%

Per file: KaStringAsync.cs 49/49 to 60/60; KaChar.cs 28/33 to 28/28; KaKey.cs 28/33 to 28/28; IKbdAction.cs 0/0 (interface-only, no executable line). KaChar.cs and KaKey.cs reach 100% by shedding the five uncovered lines each that the removed dead API occupied.

The blocking gates pass: new production line coverage 12/12 = 100% against a 90% requirement, and no changed-line regression. The repository-wide rates remain below the 80% and 85% floors; that shortfall was measured before any edit in this change, is pre-existing and unadjudicated, and both rates moved upward here.

Review outcome

Feature review produced policy-audit, code-review, and feature-audit artifacts dated 2026-08-22T11-30, all committed.

• Blocking findings: 0. Blocking-PARTIAL: 0. Advisory: 4.
• All 21 acceptance criteria PASS, checked off in spec.md (21 checked, 0 remaining).
• No remediation cycle was required.

The four advisory findings are recorded and dispositioned non-blocking: coverage-delta flipped lines are not localized; AC18's literal text diverges from the agent-memory carve-out the plan grants at P4-T3; two pre-existing unused using directives survive in KaChar.cs and IKbdAction.cs; and the repository-wide coverage shortfall predates this change.

One arithmetic discrepancy was disclosed rather than smoothed: lines-valid reconciles exactly (+11 −5 −5 = +1) but lines-covered moved +6 where the in-scope files account for +11. Review adjudicated the disclosure adequate and not masking a regression — all four in-scope files measure 100% per file after the change, the magnitude is 0.006 percentage points, within the known dotnet-coverage run-to-run band, and the instrumented run reproduced 6441/6441 exactly.

Scope constraints honoured

• QuickFiler.Test/QuickFiler.Test.csproj is unchanged — siblings Bug: quickfiler-test-form1-live-form #491 and Bug: quickfiler-explorer-controller-latent-defects #449 own regions of it. All four new tests land in files that already carry <Compile Include> entries.
• Nothing modified under .claude/** except three .claude/agent-memory/atomic-executor/** files, which the plan explicitly carves out at P4-T3.
• Nothing under docs/features/potential/**, .github/workflows/**, config/blast-radius.json, or config/orchestration-routing.json.
• KbdActions.cs, KeyboardHandler.cs, and QfcCollectionController.cs are unmodified.
• No file exceeds 500 lines after the change; KaChar.cs, KaKey.cs, and IKbdAction.cs are each shorter than before.
• No sleep, retry, or timing tolerance was used to stabilize any test. No banned API (DateTime.Now, Random.Shared, Thread.Sleep, Task.Delay) appears in any of the five changed source files.

Files changed

Production and test sources (5):

File	Change
QuickFiler/Controllers/KaStringAsync.cs	+67 / −1 — guard clause, branch-3 gate, XML documentation
QuickFiler/Controllers/KaChar.cs	−20 — DelegateType, two Update properties, unused using
QuickFiler/Controllers/KaKey.cs	−19 — DelegateType, two Update properties (using retained; Keys is still its key type)
QuickFiler/Interfaces/IKbdAction.cs	−2 — two commented-out members
QuickFiler.Test/Controllers/KaStringAsyncTests.cs	+112 / −1 — one rename plus four new tests

The remaining 38 files are the audit trail: 30 evidence artifacts, the three review artifacts, the plan and spec checkbox updates, and three agent-memory notes.

🤖 Generated with Claude Code