From 2a60805940f82a376ece239149dd0ad081d808e0 Mon Sep 17 00:00:00 2001 From: rpickmans Date: Thu, 28 May 2026 12:02:57 -0700 Subject: [PATCH 1/2] Add AcceptanceGranularity setting for Word / Phrase / Full primary-key accept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #10. The primary accept key (default Tab) now honors a user-selected granularity: Word (current behavior), Phrase (Cursor-style — stop at .!?\n or their quoted/bracketed equivalents), or Full Suggestion (same outcome as the dedicated full-accept key). The dedicated full-accept key remains an unconditional per-press override. The new nextAcceptancePhrase chunker composes over the existing nextAcceptanceChunk so word-boundary, internal-punctuation, and leading-whitespace policy stay identical across multi-word seams. Two extra rules make sentence boundaries work robustly: - newlines are scanned inside each composed chunk (the chunker returns leading whitespace including \n, so a tail like "hello\nworld" would otherwise carry the newline forward unnoticed) - terminator detection walks back past closing quotes and brackets so "done." Next stops at the closing quote rather than over-accepting Known limitation: a tail ending in U.S.A. is treated as a sentence end (early break). Rule-based scanners can't disambiguate without NLP; Cursor and Copilot behave the same way. Per-app and per-profile granularity stay out of scope for this PR (touches the rules system — follow-up issue). --- .../SuggestionCoordinator+Acceptance.swift | 14 +- Cotabby/Models/SuggestionEngineModels.swift | 14 ++ Cotabby/Models/SuggestionSettingsModel.swift | 78 ++++++---- .../SuggestionInteractionState.swift | 25 +++- .../Support/SuggestionSessionReconciler.swift | 97 ++++++++++++ .../UI/Settings/Panes/ShortcutsPaneView.swift | 14 ++ Cotabby/UI/SettingsView.swift | 14 ++ CotabbyTests/CotabbyTestFixtures.swift | 6 +- .../SuggestionSessionReconcilerTests.swift | 139 ++++++++++++++++++ CotabbyTests/SuggestionStateHelperTests.swift | 9 +- 10 files changed, 372 insertions(+), 38 deletions(-) diff --git a/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift b/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift index 0d271830..a244319d 100644 --- a/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift +++ b/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift @@ -53,13 +53,21 @@ extension SuggestionCoordinator { ) } - let preparation = fullText - ? interactionState.prepareFullAcceptance(from: rawContext, overlayState: overlayState) - : interactionState.prepareAcceptance( + // `acceptEntireSuggestion` forces the full-acceptance path regardless of granularity so the + // dedicated full-accept key stays a per-press override. `acceptCurrentSuggestion` honors + // the user-selected granularity for the primary accept key. + let primaryGranularity = settingsSnapshot.acceptanceGranularity + let preparation: SuggestionAcceptancePreparation + if fullText || primaryGranularity == .full { + preparation = interactionState.prepareFullAcceptance(from: rawContext, overlayState: overlayState) + } else { + preparation = interactionState.prepareAcceptance( from: rawContext, overlayState: overlayState, + granularity: primaryGranularity, autoAcceptTrailingPunctuation: settingsSnapshot.autoAcceptTrailingPunctuation ) + } let liveContext: FocusedInputContext let sessionForAcceptance: ActiveSuggestionSession diff --git a/Cotabby/Models/SuggestionEngineModels.swift b/Cotabby/Models/SuggestionEngineModels.swift index 3681fbce..4366de7c 100644 --- a/Cotabby/Models/SuggestionEngineModels.swift +++ b/Cotabby/Models/SuggestionEngineModels.swift @@ -45,6 +45,17 @@ struct DisabledApplicationRule: Codable, Equatable, Identifiable, Sendable { var id: String { bundleIdentifier } } +/// How much of a buffered suggestion the primary accept key takes per press. The dedicated +/// full-accept key always takes the entire remaining tail regardless of this setting. +enum AcceptanceGranularity: String, CaseIterable, Codable, Sendable { + /// One word (with the existing trailing-punctuation policy applied per chunk). + case word + /// Words accumulated until a sentence terminator (`.`, `!`, `?`, `\n`) or the tail runs out. + case phrase + /// The entire remaining suggestion at once — same outcome as the dedicated full-accept key. + case full +} + /// A compact snapshot of the autocomplete settings the coordinator actually needs at generation /// time. Keeping this as a value type makes change detection simple and deterministic. struct SuggestionSettingsSnapshot: Equatable, Sendable { @@ -76,4 +87,7 @@ struct SuggestionSettingsSnapshot: Equatable, Sendable { /// based on caret geometry quality). Travels in the snapshot so consumers can react to changes /// without subscribing to the settings model directly. let mirrorPreference: MirrorPreference + /// How much of the buffered suggestion the primary accept key takes per press. Read once per + /// accept call so a mid-press setting change can't strand a partially-handled press. + let acceptanceGranularity: AcceptanceGranularity } diff --git a/Cotabby/Models/SuggestionSettingsModel.swift b/Cotabby/Models/SuggestionSettingsModel.swift index 573a5b88..714c9a6d 100644 --- a/Cotabby/Models/SuggestionSettingsModel.swift +++ b/Cotabby/Models/SuggestionSettingsModel.swift @@ -38,6 +38,7 @@ final class SuggestionSettingsModel: ObservableObject { @Published private(set) var fullAcceptanceKeyCode: CGKeyCode @Published private(set) var fullAcceptanceKeyModifiers: ShortcutModifierMask @Published private(set) var fullAcceptanceKeyLabel: String + @Published private(set) var acceptanceGranularity: AcceptanceGranularity private let userDefaults: UserDefaults private static let isGloballyEnabledDefaultsKey = "cotabbyGloballyEnabled" @@ -67,6 +68,7 @@ final class SuggestionSettingsModel: ObservableObject { private static let fullAcceptanceKeyCodeDefaultsKey = "cotabbyFullAcceptanceKeyCode" private static let fullAcceptanceKeyModifiersDefaultsKey = "cotabbyFullAcceptanceKeyModifiers" private static let fullAcceptanceKeyLabelDefaultsKey = "cotabbyFullAcceptanceKeyLabel" + private static let acceptanceGranularityDefaultsKey = "cotabbyAcceptanceGranularity" static let defaultAcceptanceKeyCode: CGKeyCode = 48 static let defaultAcceptanceKeyLabel = "Tab" @@ -200,6 +202,13 @@ final class SuggestionSettingsModel: ObservableObject { ) let resolvedFullAcceptanceKeyLabel = userDefaults.string(forKey: Self.fullAcceptanceKeyLabelDefaultsKey) ?? Self.defaultFullAcceptanceKeyLabel + // Default `.word` preserves the pre-feature behavior for existing installs that have no + // value persisted yet. Invalid persisted values fall back to `.word` rather than crashing + // so a hand-edited UserDefault can't strand the user. + let resolvedAcceptanceGranularity = userDefaults + .string(forKey: Self.acceptanceGranularityDefaultsKey) + .flatMap(AcceptanceGranularity.init(rawValue:)) + ?? .word isGloballyEnabled = resolvedGloballyEnabled disabledAppRules = resolvedDisabledAppRules @@ -225,6 +234,7 @@ final class SuggestionSettingsModel: ObservableObject { fullAcceptanceKeyCode = resolvedFullAcceptanceKeyCode fullAcceptanceKeyModifiers = resolvedFullAcceptanceKeyModifiers fullAcceptanceKeyLabel = resolvedFullAcceptanceKeyLabel + acceptanceGranularity = resolvedAcceptanceGranularity userDefaults.set(resolvedGloballyEnabled, forKey: Self.isGloballyEnabledDefaultsKey) persistDisabledAppRules(resolvedDisabledAppRules) @@ -253,6 +263,7 @@ final class SuggestionSettingsModel: ObservableObject { forKey: Self.fullAcceptanceKeyModifiersDefaultsKey ) userDefaults.set(resolvedFullAcceptanceKeyLabel, forKey: Self.fullAcceptanceKeyLabelDefaultsKey) + userDefaults.set(resolvedAcceptanceGranularity.rawValue, forKey: Self.acceptanceGranularityDefaultsKey) // The custom indicator icon feature was removed; scrub any previously-persisted PNG so // users who picked one in an older build get the default cat icon back automatically. @@ -279,7 +290,8 @@ final class SuggestionSettingsModel: ObservableObject { isMultiLineEnabled: isMultiLineEnabled, autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation, isFastModeEnabled: isFastModeEnabled, - mirrorPreference: mirrorPreference + mirrorPreference: mirrorPreference, + acceptanceGranularity: acceptanceGranularity ) } @@ -344,6 +356,14 @@ final class SuggestionSettingsModel: ObservableObject { userDefaults.set(enabled, forKey: Self.autoAcceptTrailingPunctuationDefaultsKey) } + func setAcceptanceGranularity(_ granularity: AcceptanceGranularity) { + guard acceptanceGranularity != granularity else { + return + } + acceptanceGranularity = granularity + userDefaults.set(granularity.rawValue, forKey: Self.acceptanceGranularityDefaultsKey) + } + func setDebounceMilliseconds(_ value: Int) { let clamped = max(10, min(500, value)) guard debounceMilliseconds != clamped else { @@ -778,7 +798,10 @@ extension SuggestionSettingsModel: SuggestionSettingsProviding { // upstreams. Group related settings into nested combiners so the shape stays readable. // `presentationToggles` carries the visual-pipeline knobs (clipboard, fast mode, mirror // preference); they share the property of "affects how/when suggestions are shown". - Publishers.CombineLatest4( + // + // The outer CombineLatest4 is at the cap, so `$acceptanceGranularity` is layered above it + // via a second CombineLatest to avoid restructuring the existing groupings. + let primary = Publishers.CombineLatest4( Publishers.CombineLatest4( $isGloballyEnabled, $disabledAppRules, @@ -794,29 +817,32 @@ extension SuggestionSettingsModel: SuggestionSettingsProviding { $autoAcceptTrailingPunctuation ) ) - .map { combinedSettings, presentationToggles, profile, timing in - let (globallyEnabled, disabledAppRules, engine, wordCountPreset) = combinedSettings - let (clipboardContextEnabled, fastModeEnabled, mirrorPreference) = presentationToggles - let (userName, customRules, responseLanguages) = profile - let (debounce, focusPoll, multiLine, autoAcceptPunctuation) = timing - return SuggestionSettingsSnapshot( - isGloballyEnabled: globallyEnabled, - disabledAppBundleIdentifiers: Set(disabledAppRules.map(\.bundleIdentifier)), - selectedEngine: engine, - selectedWordCountPreset: wordCountPreset, - isClipboardContextEnabled: clipboardContextEnabled, - userName: userName, - customRules: customRules, - responseLanguages: responseLanguages, - debounceMilliseconds: debounce, - focusPollIntervalMilliseconds: focusPoll, - isMultiLineEnabled: multiLine, - autoAcceptTrailingPunctuation: autoAcceptPunctuation, - isFastModeEnabled: fastModeEnabled, - mirrorPreference: mirrorPreference - ) - } - .removeDuplicates() - .eraseToAnyPublisher() + return Publishers.CombineLatest(primary, $acceptanceGranularity) + .map { primaryTuple, granularity in + let (combinedSettings, presentationToggles, profile, timing) = primaryTuple + let (globallyEnabled, disabledAppRules, engine, wordCountPreset) = combinedSettings + let (clipboardContextEnabled, fastModeEnabled, mirrorPreference) = presentationToggles + let (userName, customRules, responseLanguages) = profile + let (debounce, focusPoll, multiLine, autoAcceptPunctuation) = timing + return SuggestionSettingsSnapshot( + isGloballyEnabled: globallyEnabled, + disabledAppBundleIdentifiers: Set(disabledAppRules.map(\.bundleIdentifier)), + selectedEngine: engine, + selectedWordCountPreset: wordCountPreset, + isClipboardContextEnabled: clipboardContextEnabled, + userName: userName, + customRules: customRules, + responseLanguages: responseLanguages, + debounceMilliseconds: debounce, + focusPollIntervalMilliseconds: focusPoll, + isMultiLineEnabled: multiLine, + autoAcceptTrailingPunctuation: autoAcceptPunctuation, + isFastModeEnabled: fastModeEnabled, + mirrorPreference: mirrorPreference, + acceptanceGranularity: granularity + ) + } + .removeDuplicates() + .eraseToAnyPublisher() } } diff --git a/Cotabby/Services/Suggestion/SuggestionInteractionState.swift b/Cotabby/Services/Suggestion/SuggestionInteractionState.swift index 1eb6e291..d906eb14 100644 --- a/Cotabby/Services/Suggestion/SuggestionInteractionState.swift +++ b/Cotabby/Services/Suggestion/SuggestionInteractionState.swift @@ -104,9 +104,14 @@ final class SuggestionInteractionState { /// Validates whether the current stored session can be accepted from the latest live AX state. /// The returned value gives the coordinator the exact chunk to insert and the context it should /// use for diagnostics and overlay updates. + /// + /// `granularity` selects between word-by-word and phrase-by-phrase acceptance. `.full` is + /// rejected here because the coordinator routes full accepts to `prepareFullAcceptance`, which + /// keeps a distinct invalid-reason message for the dedicated full-accept key. func prepareAcceptance( from snapshot: FocusedInputSnapshot, overlayState: OverlayState, + granularity: AcceptanceGranularity, autoAcceptTrailingPunctuation: Bool = true ) -> SuggestionAcceptancePreparation { let validated = validateSessionForAcceptance(from: snapshot, overlayState: overlayState) @@ -114,10 +119,22 @@ final class SuggestionInteractionState { return .invalid(validated.failureReason ?? "Key passed through.") } - let chunk = SuggestionSessionReconciler.nextAcceptanceChunk( - from: session.remainingText, - autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation - ) + let chunk: String + switch granularity { + case .word: + chunk = SuggestionSessionReconciler.nextAcceptanceChunk( + from: session.remainingText, + autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation + ) + case .phrase: + chunk = SuggestionSessionReconciler.nextAcceptancePhrase( + from: session.remainingText, + autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation + ) + case .full: + preconditionFailure("prepareAcceptance should not receive .full — route to prepareFullAcceptance instead") + } + guard !chunk.isEmpty else { return .invalid("Key passed through because no remaining suggestion chunk was available.") } diff --git a/Cotabby/Support/SuggestionSessionReconciler.swift b/Cotabby/Support/SuggestionSessionReconciler.swift index 2400dea4..daa4b5a1 100644 --- a/Cotabby/Support/SuggestionSessionReconciler.swift +++ b/Cotabby/Support/SuggestionSessionReconciler.swift @@ -255,6 +255,87 @@ enum SuggestionSessionReconciler { return String(remainingText[.. String { + guard !remainingText.isEmpty else { + return "" + } + + var accumulated = "" + var working = remainingText + + while !working.isEmpty { + let chunk = nextAcceptanceChunk( + from: working, + autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation + ) + guard !chunk.isEmpty else { + break + } + + if let newlineIndex = chunk.firstIndex(of: "\n") { + accumulated += chunk[...newlineIndex] + return accumulated + } + + accumulated += chunk + working = String(working.dropFirst(chunk.count)) + + if endsInSentenceTerminator(accumulated) { + return accumulated + } + } + + return accumulated + } + + /// Tail-end check for sentence terminators that survives closing quotes and brackets, so + /// `"done."` and `(yes!)` are recognized as phrase ends even though their final character is + /// a closer rather than `.!?`. Walks back past any run of closing punctuation, then checks + /// whether the character immediately before that run is a sentence terminator. + private static func endsInSentenceTerminator(_ text: String) -> Bool { + var index = text.endIndex + while index > text.startIndex { + let prev = text.index(before: index) + if text[prev].isPhraseClosingPunctuation { + index = prev + } else { + break + } + } + guard index > text.startIndex else { + return false + } + let prev = text.index(before: index) + return text[prev].isPhraseSentenceTerminator + } + /// Returns the index just past a word token's final alphanumeric character when that token has /// trailing punctuation worth splitting off. Returns `nil` — meaning "accept the whole token" — /// for punctuation-only tokens and for words that already end in an alphanumeric character. @@ -381,4 +462,20 @@ private extension Character { var isAcceptanceWordCharacter: Bool { isLetter || isNumber } + + /// Sentence-ending punctuation for phrase mode. `\n` is handled separately because it can + /// appear inside a leading-whitespace prefix of a composed chunk rather than at the chunk's + /// tail end. + var isPhraseSentenceTerminator: Bool { + self == "." || self == "!" || self == "?" + } + + /// Closing punctuation that may follow a sentence terminator in prose: straight + curly + /// quotes, parentheses, square brackets, and braces. The phrase scanner walks back past a + /// run of these to find the real sentence terminator underneath, so `"done."` stops as a + /// complete sentence even though its final character is the closing quote. + var isPhraseClosingPunctuation: Bool { + self == "\"" || self == "'" || self == ")" || self == "]" || self == "}" + || self == "\u{201D}" || self == "\u{2019}" + } } diff --git a/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift b/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift index 495ec264..e6062e19 100644 --- a/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift +++ b/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift @@ -11,9 +11,23 @@ struct ShortcutsPaneView: View { @State private var isRecordingKeybind = false @State private var isRecordingFullAcceptKeybind = false + private var acceptanceGranularityBinding: Binding { + Binding( + get: { suggestionSettings.acceptanceGranularity }, + set: { suggestionSettings.setAcceptanceGranularity($0) } + ) + } + var body: some View { SettingsPaneScaffold { Section("Shortcuts") { + Picker("Acceptance Mode", selection: acceptanceGranularityBinding) { + Text("Word").tag(AcceptanceGranularity.word) + Text("Phrase").tag(AcceptanceGranularity.phrase) + Text("Full Suggestion").tag(AcceptanceGranularity.full) + } + .pickerStyle(.menu) + LabeledContent("Accept Word") { KeybindRow( label: suggestionSettings.acceptanceKeyLabel, diff --git a/Cotabby/UI/SettingsView.swift b/Cotabby/UI/SettingsView.swift index 1cd8973f..4a3a3e0d 100644 --- a/Cotabby/UI/SettingsView.swift +++ b/Cotabby/UI/SettingsView.swift @@ -283,6 +283,13 @@ struct SettingsView: View { @ViewBuilder private var shortcutsSection: some View { Section("Shortcuts") { + Picker("Acceptance Mode", selection: acceptanceGranularityBinding) { + Text("Word").tag(AcceptanceGranularity.word) + Text("Phrase").tag(AcceptanceGranularity.phrase) + Text("Full Suggestion").tag(AcceptanceGranularity.full) + } + .pickerStyle(.menu) + LabeledContent("Accept Word") { HStack(spacing: 8) { Text(suggestionSettings.acceptanceKeyLabel) @@ -725,6 +732,13 @@ struct SettingsView: View { ) } + private var acceptanceGranularityBinding: Binding { + Binding( + get: { suggestionSettings.acceptanceGranularity }, + set: { suggestionSettings.setAcceptanceGranularity($0) } + ) + } + private var debounceMillisecondsBinding: Binding { Binding( get: { suggestionSettings.debounceMilliseconds }, diff --git a/CotabbyTests/CotabbyTestFixtures.swift b/CotabbyTests/CotabbyTestFixtures.swift index 2d2d1f2f..033fd8ee 100644 --- a/CotabbyTests/CotabbyTestFixtures.swift +++ b/CotabbyTests/CotabbyTestFixtures.swift @@ -221,7 +221,8 @@ enum CotabbyTestFixtures { isMultiLineEnabled: Bool = false, autoAcceptTrailingPunctuation: Bool = true, isFastModeEnabled: Bool = false, - mirrorPreference: MirrorPreference = .auto + mirrorPreference: MirrorPreference = .auto, + acceptanceGranularity: AcceptanceGranularity = .word ) -> SuggestionSettingsSnapshot { SuggestionSettingsSnapshot( isGloballyEnabled: isGloballyEnabled, @@ -237,7 +238,8 @@ enum CotabbyTestFixtures { isMultiLineEnabled: isMultiLineEnabled, autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation, isFastModeEnabled: isFastModeEnabled, - mirrorPreference: mirrorPreference + mirrorPreference: mirrorPreference, + acceptanceGranularity: acceptanceGranularity ) } } diff --git a/CotabbyTests/SuggestionSessionReconcilerTests.swift b/CotabbyTests/SuggestionSessionReconcilerTests.swift index fe0bf5d8..f5af4702 100644 --- a/CotabbyTests/SuggestionSessionReconcilerTests.swift +++ b/CotabbyTests/SuggestionSessionReconcilerTests.swift @@ -131,6 +131,145 @@ final class SuggestionSessionReconcilerTests: XCTestCase { ) } + // MARK: - Phrase chunker + + func test_nextAcceptancePhrase_returnsEmptyForEmptyTail() { + XCTAssertEqual(SuggestionSessionReconciler.nextAcceptancePhrase(from: ""), "") + } + + func test_nextAcceptancePhrase_returnsWholeTailWhenNoTerminatorPresent() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "hello world again"), + "hello world again" + ) + } + + func test_nextAcceptancePhrase_stopsAtFirstPeriod() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "hello world. foo bar."), + "hello world." + ) + } + + func test_nextAcceptancePhrase_stopsAtFirstQuestionMark() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "how are you? fine."), + "how are you?" + ) + } + + func test_nextAcceptancePhrase_stopsAtFirstExclamation() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "stop! go back"), + "stop!" + ) + } + + func test_nextAcceptancePhrase_stopsAtNewlineBetweenTokens() { + // Composition over the word chunker would otherwise carry the newline as leading whitespace + // into the next iteration's accumulated chunk; the in-chunk newline scan must catch it. + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "hello\nworld"), + "hello\n" + ) + } + + func test_nextAcceptancePhrase_stopsAtLeadingNewline() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "\nworld"), + "\n" + ) + } + + func test_nextAcceptancePhrase_stopsAtFirstOfMultipleNewlines() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "\n\nbody"), + "\n" + ) + } + + func test_nextAcceptancePhrase_includesLeadingWhitespaceUpToTerminator() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: " hello. world."), + " hello." + ) + } + + func test_nextAcceptancePhrase_preservesInteriorPunctuationWithinTokens() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "don't go. yes"), + "don't go." + ) + } + + func test_nextAcceptancePhrase_abbreviationFalseBreakIsKnownLimitation() { + // "U.S.A." ends in a period, which the rule-based scanner treats as a sentence terminator. + // The user accepts the abbreviation in one press and the next phrase begins with " is". + // Without NLP this collapse is unavoidable; Cursor and Copilot behave the same way. + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "U.S.A. is great."), + "U.S.A." + ) + } + + func test_nextAcceptancePhrase_isInvariantToAutoAcceptTrailingPunctuationFlag() { + let tail = "you? Yes." + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: tail, autoAcceptTrailingPunctuation: true), + "you?" + ) + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: tail, autoAcceptTrailingPunctuation: false), + "you?" + ) + } + + func test_nextAcceptancePhrase_stopsAtNewlineEvenWhenPunctuationPrecedes() { + // The newline must win over a sentence-terminator on the same line so paragraph breaks are + // never accidentally skipped past. + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "hello world\nmore"), + "hello world\n" + ) + } + + func test_nextAcceptancePhrase_stopsAtSentenceEndInsideStraightQuotes() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "\"done.\" Next sentence."), + "\"done.\"" + ) + } + + func test_nextAcceptancePhrase_stopsAtSentenceEndInsideCurlyQuotes() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "\u{201C}done.\u{201D} Next."), + "\u{201C}done.\u{201D}" + ) + } + + func test_nextAcceptancePhrase_stopsAtSentenceEndInsideParentheses() { + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "(yes!) keep going"), + "(yes!)" + ) + } + + func test_nextAcceptancePhrase_walksPastMultipleClosingPunctuation() { + // Nested closers — quote inside parens, sentence ends inside both. + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "(\"done.\") next"), + "(\"done.\")" + ) + } + + func test_nextAcceptancePhrase_doesNotBreakOnBareClosingQuote() { + // Closing quote with no preceding sentence terminator is not a phrase boundary. + XCTAssertEqual( + SuggestionSessionReconciler.nextAcceptancePhrase(from: "\"hi\" there"), + "\"hi\" there" + ) + } + func test_insertionChunk_dropsLeadingSpaceWhenPrecedingTextAlreadyEndsInWhitespace() { XCTAssertEqual( SuggestionSessionReconciler.insertionChunk(forAcceptedChunk: " you", precedingText: "How are "), diff --git a/CotabbyTests/SuggestionStateHelperTests.swift b/CotabbyTests/SuggestionStateHelperTests.swift index 208ceb1c..479387e1 100644 --- a/CotabbyTests/SuggestionStateHelperTests.swift +++ b/CotabbyTests/SuggestionStateHelperTests.swift @@ -127,7 +127,8 @@ final class SuggestionInteractionStateTests: XCTestCase { text: " world again", geometry: CotabbyTestFixtures.overlayGeometry(caretRect: context.caretRect), mode: .inline - ) + ), + granularity: .word ) guard case let .ready(liveContext, session, acceptedChunk) = preparation else { @@ -152,7 +153,8 @@ final class SuggestionInteractionStateTests: XCTestCase { text: " different", geometry: CotabbyTestFixtures.overlayGeometry(caretRect: context.caretRect), mode: .inline - ) + ), + granularity: .word ) guard case let .invalid(reason) = preparation else { @@ -180,7 +182,8 @@ final class SuggestionInteractionStateTests: XCTestCase { processIdentifier: 123, precedingText: "Different live text" ), - overlayState: .hidden(reason: "waiting for caret sync") + overlayState: .hidden(reason: "waiting for caret sync"), + granularity: .word ) guard case let .ready(_, _, acceptedChunk) = preparation else { From 24233cfcdf349b1d63c840537233fd3a8b60f90e Mon Sep 17 00:00:00 2001 From: rpickmans Date: Thu, 28 May 2026 15:15:16 -0700 Subject: [PATCH 2/2] Harden AcceptanceGranularity: graceful .full path, shared picker, phrase test Addresses review feedback on #388: - prepareAcceptance: replace preconditionFailure on the .full arm with assertionFailure + an .invalid return so a future mis-route degrades gracefully in release instead of crashing, matching the other .invalid branches. - Extract the duplicated Acceptance Mode picker and binding from SettingsView and ShortcutsPaneView into a shared AcceptanceModePickerView. - Add an integration test driving prepareAcceptance(granularity: .phrase) through nextAcceptancePhrase. --- Cotabby.xcodeproj/project.pbxproj | 4 +++ .../SuggestionInteractionState.swift | 6 +++- .../Components/AcceptanceModePickerView.swift | 26 +++++++++++++++++ .../UI/Settings/Panes/ShortcutsPaneView.swift | 14 +-------- Cotabby/UI/SettingsView.swift | 14 +-------- CotabbyTests/SuggestionStateHelperTests.swift | 29 +++++++++++++++++++ 6 files changed, 66 insertions(+), 27 deletions(-) create mode 100644 Cotabby/UI/Settings/Components/AcceptanceModePickerView.swift diff --git a/Cotabby.xcodeproj/project.pbxproj b/Cotabby.xcodeproj/project.pbxproj index a027ac87..e563db65 100644 --- a/Cotabby.xcodeproj/project.pbxproj +++ b/Cotabby.xcodeproj/project.pbxproj @@ -159,6 +159,7 @@ D5CAF3B590E5EC2AFC72E57A /* VisualContextStartCoalescerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 050D929E13BE52E6282B64D2 /* VisualContextStartCoalescerTests.swift */; }; D9C51DEDF01033E276A479CE /* AXTextGeometryResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = CB58035EFFD65B767949BAE6 /* AXTextGeometryResolver.swift */; }; DD7FA343F1C21C4569F6D181 /* ScreenshotContextGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B84BAE361626891F19DC9DB /* ScreenshotContextGenerator.swift */; }; + DDEDCBAA2196303455F6926A /* AcceptanceModePickerView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E5DAF68AEBFE334F68A65E82 /* AcceptanceModePickerView.swift */; }; DE236C9285635C686D66A2F6 /* TerminalAppDetectorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 43E37A7E835D3BDE6265843C /* TerminalAppDetectorTests.swift */; }; E17CAA453B1F534D284F0D89 /* PermissionHostApp.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6ACCB12E4DB32D2F2BEA567 /* PermissionHostApp.swift */; }; E313639E71AE1374D2B9A956 /* SuggestionWorkController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6B2D97BAA3618A7D0357AC44 /* SuggestionWorkController.swift */; }; @@ -348,6 +349,7 @@ E217A66717D78E1E49350EC8 /* DownloadOutcomeClassifierTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadOutcomeClassifierTests.swift; sourceTree = ""; }; E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CaretGeometrySelector.swift; sourceTree = ""; }; E43E587E421AF544A8300CE4 /* CustomRulesCatalog.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomRulesCatalog.swift; sourceTree = ""; }; + E5DAF68AEBFE334F68A65E82 /* AcceptanceModePickerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AcceptanceModePickerView.swift; sourceTree = ""; }; E6423D6CC8CC371D2DA899DE /* PermissionOverlayTracker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionOverlayTracker.swift; sourceTree = ""; }; E7F42112F14026E6253BB865 /* PermissionAndContextModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionAndContextModelTests.swift; sourceTree = ""; }; EAAE6B395FAB604DF059280A /* KeyCodeLabels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyCodeLabels.swift; sourceTree = ""; }; @@ -503,6 +505,7 @@ 6E1171BBC9CB74DB623C5E8B /* Components */ = { isa = PBXGroup; children = ( + E5DAF68AEBFE334F68A65E82 /* AcceptanceModePickerView.swift */, 19BE12C28A4AB8A4A58C2FF7 /* SettingsPaneScaffold.swift */, ); path = Components; @@ -830,6 +833,7 @@ 30F3F2B6D13CD583136CD787 /* AXHelper.swift in Sources */, D9C51DEDF01033E276A479CE /* AXTextGeometryResolver.swift in Sources */, 78FAE5DB691A1B71042B9D20 /* AboutPaneView.swift in Sources */, + DDEDCBAA2196303455F6926A /* AcceptanceModePickerView.swift in Sources */, 0A658BF137DBD0898E40B87F /* AcknowledgementsView.swift in Sources */, 26E0331E9E2F92FAE531BDEE /* ActivationIndicatorController.swift in Sources */, 0A3443AEE6540F11E5E6BF8F /* AppDelegate.swift in Sources */, diff --git a/Cotabby/Services/Suggestion/SuggestionInteractionState.swift b/Cotabby/Services/Suggestion/SuggestionInteractionState.swift index d906eb14..e04de21f 100644 --- a/Cotabby/Services/Suggestion/SuggestionInteractionState.swift +++ b/Cotabby/Services/Suggestion/SuggestionInteractionState.swift @@ -132,7 +132,11 @@ final class SuggestionInteractionState { autoAcceptTrailingPunctuation: autoAcceptTrailingPunctuation ) case .full: - preconditionFailure("prepareAcceptance should not receive .full — route to prepareFullAcceptance instead") + // The coordinator routes full accepts to `prepareFullAcceptance`, so `.full` should never + // reach here. Trap in debug, but degrade gracefully in release by passing the key through + // rather than crashing a shipping process — matching the other `.invalid` branches here. + assertionFailure("prepareAcceptance should not receive .full — route to prepareFullAcceptance instead") + return .invalid("Key passed through because .full granularity was routed incorrectly.") } guard !chunk.isEmpty else { diff --git a/Cotabby/UI/Settings/Components/AcceptanceModePickerView.swift b/Cotabby/UI/Settings/Components/AcceptanceModePickerView.swift new file mode 100644 index 00000000..9a25edf2 --- /dev/null +++ b/Cotabby/UI/Settings/Components/AcceptanceModePickerView.swift @@ -0,0 +1,26 @@ +import SwiftUI + +/// File overview: +/// Shared "Acceptance Mode" picker for the primary accept key. Used by both the legacy +/// `SettingsView.shortcutsSection` and the redesigned `ShortcutsPaneView`, so the granularity +/// labels and cases live in one place — adding or renaming a mode can't drift between the two +/// settings shells. +struct AcceptanceModePickerView: View { + @ObservedObject var suggestionSettings: SuggestionSettingsModel + + private var acceptanceGranularityBinding: Binding { + Binding( + get: { suggestionSettings.acceptanceGranularity }, + set: { suggestionSettings.setAcceptanceGranularity($0) } + ) + } + + var body: some View { + Picker("Acceptance Mode", selection: acceptanceGranularityBinding) { + Text("Word").tag(AcceptanceGranularity.word) + Text("Phrase").tag(AcceptanceGranularity.phrase) + Text("Full Suggestion").tag(AcceptanceGranularity.full) + } + .pickerStyle(.menu) + } +} diff --git a/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift b/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift index e6062e19..221ced6e 100644 --- a/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift +++ b/Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift @@ -11,22 +11,10 @@ struct ShortcutsPaneView: View { @State private var isRecordingKeybind = false @State private var isRecordingFullAcceptKeybind = false - private var acceptanceGranularityBinding: Binding { - Binding( - get: { suggestionSettings.acceptanceGranularity }, - set: { suggestionSettings.setAcceptanceGranularity($0) } - ) - } - var body: some View { SettingsPaneScaffold { Section("Shortcuts") { - Picker("Acceptance Mode", selection: acceptanceGranularityBinding) { - Text("Word").tag(AcceptanceGranularity.word) - Text("Phrase").tag(AcceptanceGranularity.phrase) - Text("Full Suggestion").tag(AcceptanceGranularity.full) - } - .pickerStyle(.menu) + AcceptanceModePickerView(suggestionSettings: suggestionSettings) LabeledContent("Accept Word") { KeybindRow( diff --git a/Cotabby/UI/SettingsView.swift b/Cotabby/UI/SettingsView.swift index 4a3a3e0d..ca9f85b7 100644 --- a/Cotabby/UI/SettingsView.swift +++ b/Cotabby/UI/SettingsView.swift @@ -283,12 +283,7 @@ struct SettingsView: View { @ViewBuilder private var shortcutsSection: some View { Section("Shortcuts") { - Picker("Acceptance Mode", selection: acceptanceGranularityBinding) { - Text("Word").tag(AcceptanceGranularity.word) - Text("Phrase").tag(AcceptanceGranularity.phrase) - Text("Full Suggestion").tag(AcceptanceGranularity.full) - } - .pickerStyle(.menu) + AcceptanceModePickerView(suggestionSettings: suggestionSettings) LabeledContent("Accept Word") { HStack(spacing: 8) { @@ -732,13 +727,6 @@ struct SettingsView: View { ) } - private var acceptanceGranularityBinding: Binding { - Binding( - get: { suggestionSettings.acceptanceGranularity }, - set: { suggestionSettings.setAcceptanceGranularity($0) } - ) - } - private var debounceMillisecondsBinding: Binding { Binding( get: { suggestionSettings.debounceMilliseconds }, diff --git a/CotabbyTests/SuggestionStateHelperTests.swift b/CotabbyTests/SuggestionStateHelperTests.swift index 479387e1..cf35f659 100644 --- a/CotabbyTests/SuggestionStateHelperTests.swift +++ b/CotabbyTests/SuggestionStateHelperTests.swift @@ -194,6 +194,35 @@ final class SuggestionInteractionStateTests: XCTestCase { } } + func test_prepareAcceptance_phraseGranularityAcceptsThroughSentenceTerminator() { + runOnMainActor { + let state = makeState() + let context = CotabbyTestFixtures.focusedInputContext(precedingText: "Hello") + _ = state.startSession(fullText: " hello world. next", liveContext: context, latency: 0.1) + let snapshot = CotabbyTestFixtures.focusedInputSnapshot(precedingText: "Hello") + + let preparation = state.prepareAcceptance( + from: snapshot, + overlayState: .visible( + text: " hello world. next", + geometry: CotabbyTestFixtures.overlayGeometry(caretRect: context.caretRect), + mode: .inline + ), + granularity: .phrase + ) + + guard case let .ready(_, session, acceptedChunk) = preparation else { + XCTFail("Expected phrase acceptance to be ready") + return + } + // Phrase mode must delegate to nextAcceptancePhrase: the accepted chunk spans every word + // up to the sentence terminator, not just the first word a .word accept would take. + XCTAssertEqual(session.remainingText, " hello world. next") + XCTAssertEqual(acceptedChunk, " hello world.") + XCTAssertEqual(SuggestionSessionReconciler.nextAcceptanceChunk(from: session.remainingText), " hello") + } + } + func test_commitAcceptedChunkAdvancesSessionAndSetsPendingInsertionSentinel() { runOnMainActor { let state = makeState()