Skip to content

Fix QuickFiler navigation key collisions during collection rebuild - #112

Merged
drmoisan merged 4 commits into
developmentfrom
bug/quickfiler-navigation-key-collision-111
Mar 28, 2026
Merged

Fix QuickFiler navigation key collisions during collection rebuild#112
drmoisan merged 4 commits into
developmentfrom
bug/quickfiler-navigation-key-collision-111

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

Fix QuickFiler navigation key collisions during collection rebuild

Summary

  • Fixes the QuickFiler bug where KbdActions could throw System.ArgumentException during collection navigation rebuild because stored keys were treated as duplicates by substring matching instead of exact stored-key identity.
  • Keeps runtime keyboard matching behavior available for lookup and filtering while separating storage identity from KaStringAsync.KeyEquals.
  • Adds focused MSTest coverage for the fixed collision path, the exact-duplicate guard, and the preserved keyboard-matching compatibility path in QuickFiler.Test/Controllers/KbdActionsTests.cs.
  • Updates QuickFiler.Test/QuickFiler.Test.csproj to include the new regression test file in the test assembly.
  • Includes the feature issue, plan, audit, regression evidence, and QA-gate artifacts under docs/features/active/2026-03-27-quickfiler-navigation-key-collision-111/.
  • Adds docs/features/potential/promoted/.gitkeep to preserve the promoted-folder path expected by tooling.

Why

QuickFiler could fail while rebuilding keyboard navigation after collection items were removed or re-registered. The issue document records that KbdActions reused KaStringAsync.KeyEquals substring matching for stored-key identity, which caused distinct literal keys such as 1, 01, and 10 for the same SourceId to collide during storage operations.

The approved plan constrained the fix to a minimal change: storage operations in KbdActions must distinguish exact stored keys, but runtime keyboard-input matching must remain available for filtering and lookup behavior. The acceptance criteria required four outcomes:

  • distinct stored keys must no longer collide during storage;
  • exact duplicates must still throw ArgumentException;
  • runtime keyboard matching semantics must remain available;
  • the repository C# QA loop must pass.

What Changed

  • Core behavior / architecture

    • Updated QuickFiler/Controllers/KbdActions.cs so storage operations use exact stored-key identity instead of substring-based matching for duplicate detection and removal.
    • Kept runtime matching behavior available for lookup/filter operations that still rely on KeyEquals.
    • No compatibility edit was required in QuickFiler/Controllers/QfcCollectionController.cs.
  • Tests

    • Added QuickFiler.Test/Controllers/KbdActionsTests.cs with focused regressions covering:
      • distinct stored keys such as 10 and 1 coexisting for SourceId = "Collection";
      • exact duplicate stored keys still throwing ArgumentException;
      • preserved keyboard-matching semantics for filtering/lookup behavior.
    • Updated QuickFiler.Test/QuickFiler.Test.csproj so the new test file is included in the test project.
  • Docs / audit / evidence

    • Added the active feature folder for Issue #111, including issue.md, plan.2026-03-27T12-45.md, regression evidence, QA-gate evidence, and audit artifacts.
    • Added docs/features/potential/promoted/.gitkeep to keep the promoted-folder path present for tooling.

Architecture / How It Fits Together

The production flow described in the issue remains the same:

  • QfcCollectionController.RegisterNavigationAsyncAction, RegisterNavigation, and RemovedItemMonitor rebuild collection navigation entries.
  • Those paths register entries through QuickFiler.Controllers.KbdActions.Add.
  • After this change, KbdActions uses exact stored-key identity for storage-oriented operations such as duplicate detection and removal.
  • Runtime lookup and filtering behavior still routes through KaStringAsync.KeyEquals for methods called out in the feature audit, including ContainsKey, FilterKeys, Find, and FindIndex.

This preserves the existing keyboard-input matching contract while preventing storage-time collisions between distinct literal keys.

Verification

Completed

  • Regression evidence captured before the fix in docs/features/active/2026-03-27-quickfiler-navigation-key-collision-111/evidence/regression-testing/p1-t2-kbdactions-distinct-keys.2026-03-27T13-01.md:
    • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-MSTest.ps1 -SearchRoot QuickFiler.Test -Configuration Debug
    • EXIT_CODE: 1
    • The evidence records the intended failing test: QuickFiler.Controllers.Tests.KbdActionsTests.Add_WhenSourceAndStoredKeysAreDistinct_DoesNotTreatSubstringAsDuplicate
    • Failure signal recorded: System.ArgumentException: Cannot add key because it already exists. Key 1 SourceId Collection
  • Formatter QA gate passed:
    • dotnet tool run csharpier format .
    • EXIT_CODE: 0
  • Analyzer QA gate passed:
    • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-VSBuild.ps1 -SolutionPath TaskMaster.sln -Configuration Debug -Platform 'Any CPU' -EnableNETAnalyzers -EnforceCodeStyleInBuild
    • EXIT_CODE: 0
  • Nullable/type-check QA gate passed:
    • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-VSBuild.ps1 -SolutionPath TaskMaster.sln -Configuration Debug -Platform 'Any CPU' -EnableNullable -TreatWarningsAsErrors
    • EXIT_CODE: 0
  • Coverage-enabled MSTest QA gate passed:
    • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-MSTestWithCoverage.ps1 -SearchRoot . -Configuration Debug
    • EXIT_CODE: 0
    • Recorded result: 2877 total tests, 2875 passed, 2 skipped, 0 failed
    • Recorded overall line coverage: 61.61%
    • Recorded QuickFiler line coverage: 22.50%

Recommended

  • dotnet tool run csharpier format .
  • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-VSBuild.ps1 -SolutionPath TaskMaster.sln -Configuration Debug -Platform 'Any CPU' -EnableNETAnalyzers -EnforceCodeStyleInBuild
  • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-VSBuild.ps1 -SolutionPath TaskMaster.sln -Configuration Debug -Platform 'Any CPU' -EnableNullable -TreatWarningsAsErrors
  • pwsh -NoProfile -ExecutionPolicy Bypass -File scripts/vscode/Invoke-MSTestWithCoverage.ps1 -SearchRoot . -Configuration Debug

Backward Compatibility / Migration Notes

  • No public API or migration step is documented for this change.
  • QfcCollectionController behavior remains compatible without source changes.
  • The intended behavioral change is limited to storage identity in KbdActions: distinct stored keys are no longer treated as duplicates solely because one literal key contains another.
  • Runtime keyboard-input matching semantics are intentionally preserved for lookup and filtering behavior.

Risks and Mitigations

  • Risk: Storage identity and runtime matching now rely on different comparison behaviors, which could introduce regressions if the distinction is misunderstood later.
    Mitigation: The new tests explicitly cover both exact stored-key identity and preserved runtime matching behavior.
  • Risk: The focused regression names 10 and 1, while the issue also mentions 01.
    Mitigation: The feature audit notes that exact string equality generalizes to the 01 case, and a future hardening pass can add a direct literal regression if desired.
  • Risk: This PR includes substantial documentation and audit artifacts alongside the code change.
    Mitigation: Review the production and test deltas first, then inspect evidence and audit files as supporting material.

Review Guide

  • Review docs/features/active/2026-03-27-quickfiler-navigation-key-collision-111/issue.md for the bug statement, reproduction path, and acceptance criteria.
  • Review QuickFiler/Controllers/KbdActions.cs for the storage-identity change.
  • Review QuickFiler.Test/Controllers/KbdActionsTests.cs for the new regression, duplicate-guard, and compatibility scenarios.
  • Review QuickFiler.Test/QuickFiler.Test.csproj for the test-project inclusion update.
  • Review docs/features/active/2026-03-27-quickfiler-navigation-key-collision-111/evidence/regression-testing/p1-t2-kbdactions-distinct-keys.2026-03-27T13-01.md for the recorded pre-fix failure.
  • Review the QA-gate artifacts under docs/features/active/2026-03-27-quickfiler-navigation-key-collision-111/evidence/qa-gates/ for the clean-pass formatter, analyzer, nullable, and coverage-enabled test loop.
  • Review the audit artifacts in the same feature folder only after the code/test changes if you want the acceptance-criteria and policy traceability.

Follow-ups

  • Optionally refresh artifacts/pr_context.summary.txt and artifacts/pr_context.appendix.txt again before merge if the PR-context collector is rerun for the final branch state.
  • Optionally add a direct "01" literal regression in a future hardening pass.
  • Optionally add a focused Remove() regression in a future hardening pass.

GitHub Auto-close

Related issues / PRs

  • None

@drmoisan
drmoisan merged commit b4ba8d1 into development Mar 28, 2026
0 of 2 checks passed
@drmoisan
drmoisan deleted the bug/quickfiler-navigation-key-collision-111 branch March 28, 2026 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant