Add user notifications for Kanata status #1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| @Published private(set) var currentState: SimpleKanataState = .starting { | ||
| didSet { | ||
| if oldValue != currentState { | ||
| UserNotificationService.shared.notifyStatusChange(currentState) |
There was a problem hiding this comment.
[P0] Call main-actor notification API from non-isolated KanataManager
UserNotificationService is declared @MainActor, so invoking UserNotificationService.shared.notifyStatusChange(...) from KanataManager’s property observer (which is not main-actor isolated) will not compile: the compiler reports “call to main actor-isolated instance method … in a synchronous nonisolated context.” Wrap these calls in Task { @MainActor in … } or mark KanataManager as @MainActor before accessing the notification service. The same issue exists in the launchFailureStatus observer below.
Useful? React with 👍 / 👎.
malpern
deleted the
codex/add-usernotificationservice-for-notifications
branch
… SystemValidator **Big Win #1: Validation Consolidation** The wizard was already migrated to use SystemValidator in Phase 2. This commit completes the cleanup by removing the now-unused SystemStatusChecker entirely. **What was removed:** - SystemStatusChecker.swift (934 lines, 51 functions) - Unused resetSharedInstance() call in WizardAutoFixer - Integration test for SystemStatusChecker (behavior no longer relevant) **What replaced it:** - SystemValidator (223 lines) - stateless, defensive assertions - SystemSnapshotAdapter - converts SystemValidator output to wizard format - Already in use by WizardStateManager since Phase 2 **Benefits:** - 934 lines removed (2.3% codebase reduction) - Single validation system for entire app (no duplicate logic) - Clearer architecture: SystemValidator for validation, Adapter for UI presentation - No functionality lost - wizard already using SystemValidator **Testing:** - Build succeeds - Wizard validation logic unchanged (same underlying implementation) - Test updated to remove obsolete integration test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed all critical bugs, warnings, and info-level issues identified in post-implementation code review. Critical Fixes: - Fix #1: Correct documentation - first group wins (not last) - Fix #2: Use stable UUIDs in Ben Vallack preset for equality checks - Fix #3: Rename isValidCombo → isRecommendedCombo (semantic clarity) Warning Fixes: - Fix #4: Document hasCrossGroupConflicts O(n×m) performance - Fix #5: Document ChordSpeed.nearest tie behavior (first/faster wins) - Fix #6: Add ASCII-only validation for group names (Kanata compat) - Fix #7: Document QWERTY layout assumption in ergonomic scoring - Fix #8: Align ChordCategory/ChordSpeed timeout values - Fix #9: Add conflict resolution helpers (conflictingGroups/Keys, wouldCreateConflict) - Fix #10: Fix overlapping detection - allow single+multi key combos Info Fixes: - Fix #11: Document O(n²) complexity in detectConflicts - Fix #12: Add public inits for ChordConflict/CrossGroupConflict - Fix #13: Add comprehensive doc comments with examples - Fix #16: Document 50-5000ms timeout range rationale Missing Tests: - Fix #15: Add testCrossGroupConflictGeneration (integration) - Add testSingleKeyVsMultiKeyOverlap (validation) - Update test expectations for timeout alignment - Fix Unicode test to expect ASCII validation All 69 chord groups tests passing. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
UNUserNotificationCenterinUserNotificationServiceTesting
swift test(fails: no such module 'AppKit')https://chatgpt.com/codex/tasks/task_e_68b3280ddfa0832ca3f9ca7d1731074a