diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/AdmissionVerdictPresenter.swift b/Sources/KeyPathAppKit/Services/RuleCollections/AdmissionVerdictPresenter.swift index d3182d44e..2b6179640 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/AdmissionVerdictPresenter.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/AdmissionVerdictPresenter.swift @@ -30,8 +30,8 @@ enum AdmissionVerdictPresenter { let who = names.count == 2 ? "\(names[0]) and \(names[1])" : names.joined(separator: ", ") let what = details.isEmpty ? "" : " (\(details.joined(separator: "; ")))" return AdmissionPresentation( - title: "Key conflict on \(key)", - explanation: "\(who) both map \(key) on the \(layer) layer\(what). Only one can win.", + title: "Key conflict on \(keyLabel(key))", + explanation: "\(who) both map \(keyLabel(key)) on the \(layer) layer\(what). Only one can win.", options: verdict.fixes.map { option($0, name) } ) case let .unmetNeed(need): @@ -69,21 +69,21 @@ enum AdmissionVerdictPresenter { } } - /// For `automatic` verdicts: the one line worth showing afterwards. - static func oneLine(_ verdict: AdmissionVerdict, name: (AdmissionSubject) -> String) -> String { - let presentation = present(verdict, name: name) - return verdict.severity == .automatic ? presentation.explanation : presentation.title - } - static func describe(_ need: RuleEnvironmentNeed) -> String { switch need { - case let .emits(key): "sends \(key), which the current backend can't send" + case let .emits(key): "sends \(keyLabel(key)), which the current backend can't send" case .capsLockInput: "needs Caps Lock as an input" case .deviceTargeting: "targets specific keyboards, which isn't supported in driverless mode" case let .externalApp(app, _): "needs \(app) installed" } } + /// Media keys and Caps Lock by their names; everything else as Kanata says it. + static func keyLabel(_ kanataKey: String) -> String { + if kanataKey.lowercased() == "caps" { return "Caps Lock" } + return LayerMappingBuilder.mediaKeyDisplayLabel(kanataKey) ?? kanataKey + } + private static func displayName(_ subject: AdmissionSubject, _ name: (AdmissionSubject) -> String) -> String { if case let .named(text) = subject { return text } return name(subject) diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictDetection.swift b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictDetection.swift index de6504d86..127f85983 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictDetection.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictDetection.swift @@ -61,22 +61,6 @@ extension RuleCollectionsManager { ) } - /// Would mappings on `keys` (Kanata names) on `layer` collide with an enabled - /// custom rule? Kept for callers that have keys rather than a collection; - /// answered through the same engine via a synthetic candidate. - func conflictWithCustomRules(_ keys: Set, layer: RuleCollectionLayer) -> RuleConflictInfo? { - let probe = RuleCollection( - id: UUID(), name: "probe", summary: "", category: .custom, - mappings: keys.sorted().map { KeyMapping(input: $0, action: .keystroke(key: $0)) }, - isEnabled: true, icon: "", tags: [], targetLayer: layer, momentaryActivator: nil, configuration: .list - ) - let collections = ruleCollections.filter { !isNeovimAppScopedReference($0) } - let state = admissionState(collections: collections + [probe], customRules: customRules) - guard let info = firstConflict(involving: .collection(probe.id), in: state), - case .customRule = info.source else { return nil } - return info - } - /// The first conflict verdict naming `subject`, folded to one other party and /// all the keys they collide on. A synthetic party (the leader key, a chord /// group) is reported too; the dialog shows it without offering to disable it. diff --git a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictResolution.swift b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictResolution.swift index 8b6345bd0..b8e91c434 100644 --- a/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictResolution.swift +++ b/Sources/KeyPathAppKit/Services/RuleCollections/RuleCollectionsManager+ConflictResolution.swift @@ -6,41 +6,6 @@ import KeyPathRulesCore extension RuleCollectionsManager { // MARK: - Save-time mapping-conflict resolution (#460) - /// Determine whether a set of save-time mapping conflicts can be resolved by - /// disabling a collection — i.e. every named party of every conflict maps to a - /// real, enabled collection by exact name. Synthetic parties (chord group names, - /// alias key-pairs, the "Leader Key" preference) won't match a collection, so - /// those conflicts fall back to a plain explanation rather than offering an - /// action that wouldn't apply. - /// - /// - Returns: the distinct collections the user could disable (≥2), or nil when - /// the conflicts aren't all collection-vs-collection. - nonisolated static func resolvableCollectionConflict( - conflicts: [KeyPathError.MappingConflictInfo], - collections: [RuleCollection] - ) -> [RuleCollection]? { - guard !conflicts.isEmpty else { return nil } - - let byName = Dictionary( - collections.filter(\.isEnabled).map { ($0.name, $0) }, - uniquingKeysWith: { first, _ in first } - ) - - var involved: [UUID: RuleCollection] = [:] - for conflict in conflicts { - // Every party must resolve to a real enabled collection; otherwise the - // whole set is treated as non-actionable (explanation fallback). - for name in conflict.conflictingCollections { - guard let collection = byName[name] else { return nil } - involved[collection.id] = collection - } - } - - // Need at least two distinct collections for "disable one" to resolve anything. - guard involved.count >= 2 else { return nil } - return involved.values.sorted { $0.name < $1.name } - } - /// On a save-time mapping-conflict failure, if the conflicting parties are all /// real collections, prompt the user to disable one and retry the save (#460). /// - Returns: the retry result, or nil when the failure wasn't a resolvable diff --git a/Sources/KeyPathAppKit/Services/SessionRuntime/SessionCapsRuntimeSupport.swift b/Sources/KeyPathAppKit/Services/SessionRuntime/SessionCapsRuntimeSupport.swift index 6377bb893..17a83a038 100644 --- a/Sources/KeyPathAppKit/Services/SessionRuntime/SessionCapsRuntimeSupport.swift +++ b/Sources/KeyPathAppKit/Services/SessionRuntime/SessionCapsRuntimeSupport.swift @@ -290,11 +290,6 @@ enum SessionCapsRuntimeSupport { return selection } - /// Without attribution; the lifecycle coordinator adds the engine's. - static func startupFailureMessage(_ result: KanataHostBridgeValidationResult) -> String? { - SessionConfigurationRefusal.message(result) - } - static func journalDirectory(configPath: String = KeyPathConstants.Config.mainConfigPath) -> URL { URL(fileURLWithPath: configPath).deletingLastPathComponent().appendingPathComponent(".session-caps-mapping", isDirectory: true) } diff --git a/Sources/KeyPathAppKit/UI/Rules/MappingConflictResolutionDialog.swift b/Sources/KeyPathAppKit/UI/Rules/MappingConflictResolutionDialog.swift index 933dad6e5..cc63dd288 100644 --- a/Sources/KeyPathAppKit/UI/Rules/MappingConflictResolutionDialog.swift +++ b/Sources/KeyPathAppKit/UI/Rules/MappingConflictResolutionDialog.swift @@ -12,8 +12,8 @@ struct MappingConflictOption: Sendable, Equatable, Identifiable { /// Context for a save-time mapping conflict that can be resolved by disabling a /// collection. Sendable so it can cross the actor boundary from the save task to -/// the UI. Built only when every conflicting party is a real, toggleable collection -/// (see `RuleCollectionsManager.resolvableCollectionConflict`). +/// the UI. Built from the engine's verdicts by `RuleCollectionsManager.mappingConflictContext(for:)`, +/// only when every conflicting party is a real, toggleable collection. struct MappingConflictContext: Sendable, Equatable { /// Plain-English explanation of the conflict(s). let explanation: String diff --git a/Tests/KeyPathTests/RuleCollections/AdmissionVerdictPresenterTests.swift b/Tests/KeyPathTests/RuleCollections/AdmissionVerdictPresenterTests.swift index 53ea16523..d7113e97a 100644 --- a/Tests/KeyPathTests/RuleCollections/AdmissionVerdictPresenterTests.swift +++ b/Tests/KeyPathTests/RuleCollections/AdmissionVerdictPresenterTests.swift @@ -41,13 +41,13 @@ final class AdmissionVerdictPresenterTests: XCTestCase { reason: .unmetNeed(.emits(key: "volu")), fixes: [.disable(.collection(UUID()))]) let p = AdmissionVerdictPresenter.present(verdict, name: names) XCTAssertEqual(p.title, "Home Row Mods can't run here") - XCTAssertEqual(p.explanation, "Home Row Mods sends volu, which the current backend can't send.") + XCTAssertEqual(p.explanation, "Home Row Mods sends Volume Up, which the current backend can't send.") } func testReservedKeyMoveIsOneLineAndBlockedFormHasAnOption() { let move = AdmissionVerdict(severity: .automatic, subject: .customRule(UUID()), reason: .reservedKey(key: "f18"), fixes: [.moveCapsIntermediate(candidates: ["f19", "f20"])]) - XCTAssertEqual(AdmissionVerdictPresenter.oneLine(move, name: names), + XCTAssertEqual(AdmissionVerdictPresenter.present(move, name: names).explanation, "My rule uses F18, which KeyPath had reserved for Caps Lock. Caps Lock will use F19 instead.") let blocked = AdmissionVerdict(severity: .blocking, subject: .customRule(UUID()), reason: .reservedKey(key: "f18"), fixes: [.disable(.customRule(UUID()))]) diff --git a/Tests/KeyPathTests/RuleCollections/RuleCollectionConflictDetectionTests.swift b/Tests/KeyPathTests/RuleCollections/RuleCollectionConflictDetectionTests.swift index 03f32b15d..c6c08510b 100644 --- a/Tests/KeyPathTests/RuleCollections/RuleCollectionConflictDetectionTests.swift +++ b/Tests/KeyPathTests/RuleCollections/RuleCollectionConflictDetectionTests.swift @@ -207,39 +207,6 @@ final class RuleCollectionConflictDetectionTests: XCTestCase { XCTAssertNotNil(conflict) } - // MARK: - conflictWithCustomRules - - func testConflictWithCustomRules_FindsMatchingKey() { - let manager = makeManager() - var rule = CustomRule(input: "x", action: .keystroke(key: "y")) - rule.isEnabled = true - manager.customRules = [rule] - - let conflict = manager.conflictWithCustomRules(Set(["x"]), layer: .base) - XCTAssertNotNil(conflict) - } - - func testConflictWithCustomRules_NoMatchWhenDifferentLayer() { - let manager = makeManager() - var rule = CustomRule(input: "x", action: .keystroke(key: "y")) - rule.isEnabled = true - rule.targetLayer = .base - manager.customRules = [rule] - - let conflict = manager.conflictWithCustomRules(Set(["x"]), layer: .navigation) - XCTAssertNil(conflict) - } - - func testConflictWithCustomRules_NoMatchWhenDisabled() { - let manager = makeManager() - var rule = CustomRule(input: "x", action: .keystroke(key: "y")) - rule.isEnabled = false - manager.customRules = [rule] - - let conflict = manager.conflictWithCustomRules(Set(["x"]), layer: .base) - XCTAssertNil(conflict) - } - // MARK: - RuleConflictInfo func testRuleConflictInfo_DisplayName_Collection() { diff --git a/Tests/KeyPathTests/RuleCollections/SaveTimeConflictResolutionTests.swift b/Tests/KeyPathTests/RuleCollections/SaveTimeConflictResolutionTests.swift deleted file mode 100644 index 7c12b36ab..000000000 --- a/Tests/KeyPathTests/RuleCollections/SaveTimeConflictResolutionTests.swift +++ /dev/null @@ -1,96 +0,0 @@ -@testable import KeyPathAppKit -import KeyPathCore -import KeyPathRulesCore -import XCTest - -/// Tests for the save-time mapping-conflict resolvability core (#460): -/// a conflict is resolvable-by-disable only when every named party maps to a -/// real, enabled collection. -final class SaveTimeConflictResolutionTests: XCTestCase { - private func collection(_ name: String, isEnabled: Bool = true) -> RuleCollection { - RuleCollection( - name: name, - summary: name, - category: .productivity, - mappings: [KeyMapping(input: "a", action: .keystroke(key: "b"))], - isEnabled: isEnabled - ) - } - - private func conflict(_ collections: [String], key: String = ";") -> KeyPathError.MappingConflictInfo { - KeyPathError.MappingConflictInfo( - inputKey: key, - layer: "Base", - conflictingCollections: collections - ) - } - - func testResolvableWhenAllPartiesAreRealCollections() { - let collections = [collection("Home Row Mods"), collection("Home Row Layer Toggles")] - let conflicts = [conflict(["Home Row Mods", "Home Row Layer Toggles"])] - - let resolvable = RuleCollectionsManager.resolvableCollectionConflict( - conflicts: conflicts, collections: collections - ) - - XCTAssertEqual(resolvable?.map(\.name), ["Home Row Layer Toggles", "Home Row Mods"]) - } - - func testNotResolvableWhenAPartyIsSynthetic() { - // Chord-group-name conflicts name "chord group '...'", which is not a collection. - let collections = [collection("Chord Groups")] - let conflicts = [conflict(["chord group 'navigation'", "chord group 'navigation'"], key: "navigation")] - - XCTAssertNil(RuleCollectionsManager.resolvableCollectionConflict( - conflicts: conflicts, collections: collections - )) - } - - func testNotResolvableWhenLeaderKeyPartyHasNoCollection() { - // The leader conflict names "Leader Key" (a preference, not a collection). - let collections = [collection("Window Mgmt")] - let conflicts = [conflict(["Leader Key", "Window Mgmt"], key: "spc")] - - XCTAssertNil(RuleCollectionsManager.resolvableCollectionConflict( - conflicts: conflicts, collections: collections - )) - } - - func testNotResolvableWhenPartyCollectionIsDisabled() { - // A disabled collection isn't in the active set, so it can't be matched. - let collections = [collection("Home Row Mods"), collection("Home Row Layer Toggles", isEnabled: false)] - let conflicts = [conflict(["Home Row Mods", "Home Row Layer Toggles"])] - - XCTAssertNil(RuleCollectionsManager.resolvableCollectionConflict( - conflicts: conflicts, collections: collections - )) - } - - func testNotResolvableWhenOnlyOneDistinctCollection() { - let collections = [collection("Home Row Mods")] - let conflicts = [conflict(["Home Row Mods", "Home Row Mods"])] - - XCTAssertNil(RuleCollectionsManager.resolvableCollectionConflict( - conflicts: conflicts, collections: collections - )) - } - - func testMixedResolvableAndSyntheticFallsBack() { - // If any conflict in the set is non-actionable, the whole set falls back. - let collections = [collection("Home Row Mods"), collection("Home Row Layer Toggles")] - let conflicts = [ - conflict(["Home Row Mods", "Home Row Layer Toggles"]), - conflict(["caps-lock / caps_lock"], key: "caps-lock / caps_lock") - ] - - XCTAssertNil(RuleCollectionsManager.resolvableCollectionConflict( - conflicts: conflicts, collections: collections - )) - } - - func testEmptyConflictsNotResolvable() { - XCTAssertNil(RuleCollectionsManager.resolvableCollectionConflict( - conflicts: [], collections: [collection("X")] - )) - } -} diff --git a/Tests/KeyPathTests/RuleCollections/SessionConfigurationRefusalTests.swift b/Tests/KeyPathTests/RuleCollections/SessionConfigurationRefusalTests.swift index d0c9cfbff..da6322a4f 100644 --- a/Tests/KeyPathTests/RuleCollections/SessionConfigurationRefusalTests.swift +++ b/Tests/KeyPathTests/RuleCollections/SessionConfigurationRefusalTests.swift @@ -43,16 +43,15 @@ final class SessionConfigurationRefusalTests: KeyPathTestCase { let message = SessionConfigurationRefusal.message(.invalid(reason: "unsupported output key volu")) XCTAssertEqual(message, "This configuration cannot run in the driverless session: unsupported output key volu. " + "Edit the configuration and retry; existing rules were preserved.") - XCTAssertEqual(message, SessionCapsRuntimeSupport.startupFailureMessage(.invalid(reason: "unsupported output key volu"))) } func testInvalidWithAttributionNamesTheRuleFirst() { let message = SessionConfigurationRefusal.message( .invalid(reason: "unsupported output key volu"), - attribution: ["Louder sends volu, which the current backend can't send."] + attribution: ["Louder sends Volume Up, which the current backend can't send."] ) XCTAssertEqual(message, "This configuration cannot run in the driverless session. " - + "Louder sends volu, which the current backend can't send. " + + "Louder sends Volume Up, which the current backend can't send. " + "Turn that rule off or change it, then retry; existing rules were preserved. " + "(Validator: unsupported output key volu.)") } @@ -70,7 +69,7 @@ final class SessionConfigurationRefusalTests: KeyPathTestCase { manager.customRules = [CustomRule(title: "Louder", input: "f12", action: .keystroke(key: "volu")), CustomRule(title: "Fine", input: "f11", action: .keystroke(key: "a"))] XCTAssertEqual(manager.blockingAdmissionSentences(profile: noMedia), - ["Louder sends volu, which the current backend can't send."]) + ["Louder sends Volume Up, which the current backend can't send."]) XCTAssertEqual(manager.blockingAdmissionSentences(profile: allowAll), []) } @@ -94,7 +93,7 @@ final class SessionConfigurationRefusalTests: KeyPathTestCase { let findings = manager.admissionFindings(profile: noMedia) XCTAssertEqual(findings.map(\.severity), ["blocking", "needsChoice", "automatic"]) XCTAssertEqual(findings[0].subject, "Louder") - XCTAssertEqual(findings[0].explanation, "Louder sends volu, which the current backend can't send.") + XCTAssertEqual(findings[0].explanation, "Louder sends Volume Up, which the current backend can't send.") XCTAssertEqual(findings[0].fixes, ["Turn off Louder"]) XCTAssertEqual(findings[1].title, "Key conflict on f13") XCTAssertEqual(Set(findings[1].fixes), ["Turn off A", "Turn off B"]) @@ -105,7 +104,7 @@ final class SessionConfigurationRefusalTests: KeyPathTestCase { func testFindingsRoundTripThroughTheControlChannel() throws { let finding = RuntimeControl.AdmissionFinding(severity: "blocking", subject: "Louder", title: "Louder can't run here", - explanation: "Louder sends volu, which the current backend can't send.", + explanation: "Louder sends Volume Up, which the current backend can't send.", fixes: ["Turn off Louder"]) var response = RuntimeControl.Response(ok: true, operation: .doctor, state: .failed, workerPID: nil, failure: "x", lastError: nil, layer: nil, appPID: 1, appVersion: "1") diff --git a/docs/architecture/rules-admission-review.md b/docs/architecture/rules-admission-review.md index 24dd912f3..f8f6ff56d 100644 --- a/docs/architecture/rules-admission-review.md +++ b/docs/architecture/rules-admission-review.md @@ -354,7 +354,7 @@ Callers, all through this one function: ### 4.4c Phase 4b (2026-10-19) - `AdmissionVerdictPresenter` words every verdict once (title, explanation, options - from its fixes; `oneLine` for automatic verdicts). Dialogs, toasts and the CLI + from its fixes). Dialogs, toasts and the CLI render from it. - The save-time dialog (`MappingConflictContext`) is now built by `RuleCollectionsManager.mappingConflictContext(for:)` from the engine's conflict @@ -433,8 +433,7 @@ Callers, all through this one function: ### 4.4f Phase 4e (2026-10-19) - **One renderer.** `SessionConfigurationRefusal.message(result, attribution:)` is the - only place "this configuration cannot run" is worded. `startupFailureMessage` delegates - to it; the lifecycle coordinator (`admitSessionConfiguration`'s error, + only place "this configuration cannot run" is worded; the lifecycle coordinator (`admitSessionConfiguration`'s error, `sessionConfigurationRefusal`, `currentRuntimeStatus`) and the save gate (`requireSessionEligibleConfiguration`) call it, so the setup screen, Status, the reload error and a refused save all say the same sentence. @@ -487,6 +486,22 @@ The detector's test corpus (`RuleCollectionDeduplicatorTests`, `MultiCollectionC unchanged, which is the proof the replacement preserves behaviour; two tests were added for what it fixes (same-titled rules told apart; details carried). +### 4.4h Cleanup (2026-10-20) + +Dead paths removed once the engine was the only implementation: +`RuleCollectionsManager.resolvableCollectionConflict` (resolved parties *by name*; the +save path has used `mappingConflictContext(for:)`, by id, since 4b) with its tests; +`conflictWithCustomRules` (no production caller since 4a); the presenter's `oneLine`; +and the `startupFailureMessage` shim that only delegated to `SessionConfigurationRefusal`. +Presenter sentences now name media keys and Caps Lock as the user knows them +("sends Volume Up", "Key conflict on Caps Lock") through the one existing table, +`LayerMappingBuilder.mediaKeyDisplayLabel`; other keys keep their Kanata names. + +`KeyPathError.MappingConflictInfo` stays, deliberately: it is the save gate's error, +carries names and details rather than ids, and lives in KeyPathCore where the verdict +types cannot be seen. It is produced from verdicts in exactly one place +(`AdmissionEngine.mappingConflicts`). + ### 4.5 Lowering: intent in rules, backend output in the generator Rules keep expressing intent (`volu`, `caps` → Hyper, "this keyboard only").