Skip to content

Refactor: kbdactions-indexer-setter-contract-decision #665

Description

@drmoisan
  • Work Mode: full-feature

Problem / Why

The indexer setter currently assigns if the key is present and silently does nothing otherwise; it never inserts. That is a legitimate design, but it is undocumented and is easy to misread as an upsert, which is exactly the misuse feature #464 had to delete from EfcItemController (issue #459). Until the contract is stated, the same misuse will recur. Feature #444, which owns KbdActions.cs, deliberately declined to change Remove's shape and left this question open.

Proposed Behavior

State the intended contract explicitly in XML documentation on the indexer, then make the implementation match it. If assign-if-present is intended, consider making the silent no-op observable (a return value or a documented precondition) so a caller cannot mistake failure for success. If upsert is intended, change the implementation and audit every call site.

Acceptance Criteria

  • The indexer setter carries XML documentation stating the chosen contract and the behavior when the key is absent.
  • A named test asserts the behavior for both the present-key and absent-key cases.
  • Every existing call site is audited against the chosen contract and any mismatch is corrected.
  • The overwriteDuplicates truth table is documented alongside the indexer contract.

Constraints & Risks

KbdActions.cs is owned by feature #444 and is consumed across several controllers, so a semantic change has a wide blast radius. Two distinct equality paths are load-bearing and must not be collapsed: Add/Remove and the duplicate guard use StoredKeyEquals (EqualityComparer<TKey>.Default), while Find/FindIndex/ContainsKey/FilterKeys use the element-defined KeyEquals. KaStringAsync.KeyEquals is substring-matching and side-effecting, and its early return must not be turned into a fall-through.

Test Conditions

  • Indexer setter with a present key, asserting assignment
  • Indexer setter with an absent key, asserting the documented behavior
  • Regression coverage over the StoredKeyEquals versus KeyEquals distinction
  • The seed-list duplicate guard still throws ArgumentException on a repeated (SourceId, StoredKey) pair

Source

From: docs/features/potential/2026-08-27-kbdactions-indexer-setter-contract-decision.md

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions