Skip to content

feat: autocomplete speaker name selection#661

Merged
r3dbars merged 3 commits into
mainfrom
feedback/speaker-autocomplete-dock-persistence
May 6, 2026
Merged

feat: autocomplete speaker name selection#661
r3dbars merged 3 commits into
mainfrom
feedback/speaker-autocomplete-dock-persistence

Conversation

@r3dbars
Copy link
Copy Markdown
Owner

@r3dbars r3dbars commented May 6, 2026

Summary

  • enables NSComboBox completion in the speaker review sheet
  • extracts speaker label sorting/completion/selection into a tested policy helper
  • preserves duplicate-name disambiguation labels while allowing unique prefix autocomplete

Test Plan

  • bash run-tests.sh
  • bash build.sh

User feedback: typing an existing speaker name should auto-complete/select it instead of requiring a dropdown click.

Copy link
Copy Markdown
Owner Author

r3dbars commented May 6, 2026

PR Review

Verdict: APPROVE

Summary: Clean extraction of speaker name selection logic from SpeakerRowView into a testable SpeakerNameSelectionPolicy enum, plus enabling NSComboBox inline autocomplete. The policy is stateless (caseless enum with static functions), well-tested, and the UI integration is minimal — one flag flip (completes = true) and delegate method wiring. Net -81 lines from the view, +129 in a focused policy file with +60 in tests.

Blockers: None.

Should fix:

  • Subtle behavior change in duplicate detection: The old makeIdentityLabels grouped duplicates by $0.displayName.lowercased(). The new version uses normalizedSearchText(displayName($0)), which adds .diacriticInsensitive folding. This means "José García" and "Jose Garcia" would now be treated as duplicates and get disambiguation labels (with call count + UUID prefix), where previously they'd be treated as distinct. This is arguably better behavior, but it's a silent change worth being aware of — confirm this is intentional.

  • "You" sentinel not in lookup: The "You" label is prepended to knownPeopleLabels (line 445) but never added to knownPeopleByLabel. If a user manually types "You" (without the batch "Keep as You" toggle), knownPeopleOption(matching: "You") returns nil, so buildUpdate() emits a .named action with newName: "You" rather than a .collapsedToMe. Verify the downstream SpeakerNamingCoordinator handles a .named update with newName: "You" gracefully — if it does, this is fine; if it expects .collapsedToMe for that case, it could create an orphan profile.

Nits:

  • Every call site and test case repeats the same id: { $0.id }, displayName: { $0.displayName }, callCount: { $0.callCount } closure triple. A test helper or a lightweight protocol conformance could cut the boilerplate, but the generic approach is reasonable for keeping the policy type-agnostic.
  • normalizedSearchText calls .folding(options: .caseInsensitive) then .lowercased(). The .lowercased() is mostly redundant after case-insensitive folding, though it serves as a safe normalization guarantee across locales — fine to keep.
  • visibleKnownPeopleLabels() is recomputed on each NSComboBoxDataSource callback (numberOfItems, objectValueForItemAt, indexOfItemWithStringValue), so the same sort runs 3+ times per keystroke. At N=2-20 speakers this is negligible, but a per-edit-cycle cache would be a cleanliness win if the list ever grows.

What looks good:

  • The completedLabel logic correctly requires exactly one unambiguous prefix match before completing — no surprising auto-completions when multiple names share a prefix.
  • Stateless enum with pure static functions is the right design here: no threading concerns, trivially testable, and the @MainActor isolation stays where it belongs (in SpeakerRowView).
  • Good test coverage: unique prefix match, ambiguous prefix refusal, and duplicate-name disambiguation all exercised.
  • The match ranking (exact → prefix → word-start → label-prefix → substring → label-substring) with call-count tiebreaking is well-thought-out and produces intuitive sort order.
  • The extraction is faithful — the logic in SpeakerNameSelectionPolicy matches the old inline implementations with no silent behavioral drift (aside from the diacritic folding note above).

Generated by Claude Code

@r3dbars r3dbars merged commit 1fd3a46 into main May 6, 2026
@r3dbars r3dbars deleted the feedback/speaker-autocomplete-dock-persistence branch May 6, 2026 10:55
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