From e0f8399ad1065800338ca87934064a05a574dcfe Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Thu, 11 Jun 2026 23:33:33 -0700 Subject: [PATCH 1/2] fix(accept): add space after every accepted word, not just the last one "Add Space After Accepting" only fired when an accept exhausted the whole suggestion. Because the model almost always returns multi-word suggestions, accepting a single word (the normal case) never exhausted it, so the toggle appeared to do nothing. Now a word-ending accept also consumes the suggestion's own following space, so the trailing space lands with this accept. Only whitespace the model already placed is taken (never synthesized), so the accepted chunk stays an exact prefix of the buffered suggestion: the session stays in lockstep with the field, the next chunk loses the leading space this one absorbed (no double space), and punctuation/newlines/space-less scripts are left untouched. The final-word case (no following space) still appends via insertionTextApplyingAutoSpace. --- .../SuggestionCoordinator+Acceptance.swift | 39 ++++++++---- .../Support/SuggestionSessionReconciler.swift | 27 +++++++++ .../SuggestionSessionReconcilerTests.swift | 60 +++++++++++++++++++ 3 files changed, 116 insertions(+), 10 deletions(-) diff --git a/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift b/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift index ee309562..4fccaf7c 100644 --- a/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift +++ b/Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift @@ -80,7 +80,11 @@ extension SuggestionCoordinator { case let .ready(preparedLiveContext, preparedSession, preparedAcceptedChunk): liveContext = preparedLiveContext sessionForAcceptance = preparedSession - acceptedChunk = preparedAcceptedChunk + // With "add space after accepting" on, a word-ending accept also takes the suggestion's + // own following space so the trailing space lands now instead of with the next accept. + // This is what makes the setting fire on every word, not only the final one that + // exhausts the suggestion (which `insertionTextApplyingAutoSpace` still covers). + acceptedChunk = autoSpaceAdjustedChunk(preparedAcceptedChunk, session: preparedSession) case let .invalid(reason): return passTabThrough(reason: reason) @@ -211,15 +215,30 @@ extension SuggestionCoordinator { } } - /// Applies the opt-in "add a space after accepting" setting to the text about to be inserted. - /// - /// The trailing space is only appended when this accept *exhausts* the suggestion — predicted the - /// same way `commitAcceptedChunk` decides it — because a mid-suggestion word accept is already - /// followed by the next chunk's own leading space, so a space here would double up. Only the - /// inserted text grows: session accounting still advances by the unchanged `acceptedChunk`, and - /// the session tears down on exhaustion, so the extra space never disturbs the consumed-suffix - /// reconciliation a still-live session relies on. Whether the space actually lands (vs. being - /// suppressed after punctuation, whitespace, or a space-less script) is the reconciler's rule. + /// Applies the opt-in "add a space after accepting" setting to the chunk being accepted, by + /// extending a word-ending chunk to also take the suggestion's own following space. The whole + /// flow (insertion text, session advance, overlay) then runs on the extended chunk, so the space + /// is consumed from the buffered suggestion rather than synthesized: the session stays in + /// lockstep with the field and the reconciler's existing rule drops the next chunk's now-leading + /// space, so there is no double space. See `acceptanceChunkConsumingTrailingSpace`. + private func autoSpaceAdjustedChunk(_ chunk: String, session: ActiveSuggestionSession) -> String { + guard settingsSnapshot.addSpaceAfterAccept else { + return chunk + } + return SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace( + chunk, + remainingText: session.remainingText + ) + } + + /// Appends the trailing space for the *final* accept of a suggestion — the one case + /// `autoSpaceAdjustedChunk` cannot cover, because an exhausting accept has no following + /// suggestion whitespace to consume. Fires only when this accept exhausts the suggestion + /// (predicted the same way `commitAcceptedChunk` decides it). Here the inserted text grows by a + /// synthesized space, but the session tears down on exhaustion, so it never disturbs the + /// consumed-suffix reconciliation a still-live session relies on. Whether the space actually + /// lands (vs. being suppressed after punctuation, whitespace, or a space-less script) is the + /// reconciler's rule. private func insertionTextApplyingAutoSpace( insertionChunk: String, acceptedChunk: String, diff --git a/Cotabby/Support/SuggestionSessionReconciler.swift b/Cotabby/Support/SuggestionSessionReconciler.swift index c8691ff0..7d01f94a 100644 --- a/Cotabby/Support/SuggestionSessionReconciler.swift +++ b/Cotabby/Support/SuggestionSessionReconciler.swift @@ -514,6 +514,33 @@ enum SuggestionSessionReconciler { return chunk + " " } + /// Extends a word-ending accepted chunk to also take the suggestion's *own* following horizontal + /// whitespace, so when "add space after accepting" is on the trailing space lands with this + /// accept rather than arriving only on the next one. This is what makes the setting fire on every + /// word of a multi-word suggestion, not just the final chunk that exhausts it. + /// + /// Only whitespace the model already placed after the word is consumed — never a synthesized + /// character — so the session stays in lockstep with the live field: the extended chunk is still + /// an exact prefix of the buffered suggestion, and the next chunk simply loses the leading space + /// this one absorbed, so no double space is produced. The chunk is returned unchanged when it + /// does not end on a finished word, ends on a space-less script, or the suggestion's next + /// character is not horizontal whitespace (punctuation stays attached to the model's own layout; + /// a newline must not be swallowed as a space; and the end-of-suggestion case is handled at + /// exhaustion by `insertionChunkAppendingTrailingSpace` instead). + /// + /// `remainingText` is the session's full remaining tail, which begins with `chunk`; the + /// whitespace examined is what follows it. + static func acceptanceChunkConsumingTrailingSpace(_ chunk: String, remainingText: String) -> String { + guard let last = chunk.last, + last.isAcceptanceWordCharacter, + !last.beginsSpacelessScriptWord else { + return chunk + } + let remainder = remainingText.dropFirst(chunk.count) + let trailingSpace = remainder.prefix(while: { $0 == " " || $0 == "\t" }) + return trailingSpace.isEmpty ? chunk : chunk + trailingSpace + } + /// Counts word-like tokens so punctuation-only accepts do not inflate productivity metrics. static func acceptedWordCount(in text: String) -> Int { text diff --git a/CotabbyTests/SuggestionSessionReconcilerTests.swift b/CotabbyTests/SuggestionSessionReconcilerTests.swift index cdb40623..acf21c24 100644 --- a/CotabbyTests/SuggestionSessionReconcilerTests.swift +++ b/CotabbyTests/SuggestionSessionReconcilerTests.swift @@ -674,6 +674,66 @@ final class SuggestionSessionReconcilerTests: XCTestCase { ) } + func test_acceptanceChunkConsumingTrailingSpace_takesFollowingSpaceAfterWord() { + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("world", remainingText: "world how are you"), + "world " + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_keepsLeadingWhitespaceAndTakesFollowingSpace() { + // nextAcceptanceChunk returns leading whitespace with the token, so the extension must keep it + // and still consume the space that follows the word. + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace(" world", remainingText: " world how"), + " world " + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_takesWholeHorizontalRun() { + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("world", remainingText: "world\t how"), + "world\t " + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_noFollowingWhitespaceLeavesChunkUntouched() { + // End of the suggestion: nothing to consume here — the exhaustion-time append covers it. + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("world", remainingText: "world"), + "world" + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_doesNotCrossNewline() { + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("line", remainingText: "line\nnext"), + "line" + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_doesNotConsumeBeforePunctuation() { + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("world", remainingText: "world, how"), + "world" + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_skipsWhenChunkEndsInPunctuation() { + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("done.", remainingText: "done. next"), + "done." + ) + } + + func test_acceptanceChunkConsumingTrailingSpace_skipsForSpacelessScript() { + // CJK scripts do not separate words with spaces, so even a stray following space is not taken. + XCTAssertEqual( + SuggestionSessionReconciler.acceptanceChunkConsumingTrailingSpace("資料", remainingText: "資料 です"), + "資料" + ) + } + func test_acceptedWordCount_countsOnlyTokensWithAlphanumerics() { let count = SuggestionSessionReconciler.acceptedWordCount( in: "hello, !!! world 123 --" From 76bcae26962ba4492dfd3ccbd51f40dcd876c2b9 Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Thu, 11 Jun 2026 23:42:47 -0700 Subject: [PATCH 2/2] test(accept): add coordinator-level integration tests for per-word auto-space Exercises the full path (autoSpaceAdjustedChunk -> insertionChunk -> commitAcceptedChunk -> state) with addSpaceAfterAccept on and off against a multi-word suggestion, so a future refactor that breaks the coordinator wiring is caught even though the pure rule is unit-tested. Addresses Greptile P2. --- ...SuggestionCoordinatorAcceptanceTests.swift | 89 ++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/CotabbyTests/SuggestionCoordinatorAcceptanceTests.swift b/CotabbyTests/SuggestionCoordinatorAcceptanceTests.swift index a66ad17a..914831fe 100644 --- a/CotabbyTests/SuggestionCoordinatorAcceptanceTests.swift +++ b/CotabbyTests/SuggestionCoordinatorAcceptanceTests.swift @@ -71,6 +71,88 @@ final class SuggestionCoordinatorAcceptanceTests: XCTestCase { } } + func test_acceptCurrentSuggestion_withAddSpaceAfterAccept_insertsTrailingSpaceOnNonFinalWord() { + // Full coordinator path with the setting ON and a multi-word suggestion: accepting the first + // word must insert the word plus the suggestion's own following space (so the toggle fires + // per word, not only when the suggestion is exhausted), while the tail keeps the rest. + runOnMainActor { + let snapshot = CotabbyTestFixtures.focusedInputSnapshot(precedingText: "Hello") + let context = FocusedInputContext(snapshot: snapshot, generation: 7) + let interactionState = SuggestionInteractionState() + let session = interactionState.startSession( + fullText: " world how", + liveContext: context, + latency: 0.1 + ) + let overlayState = OverlayState.visible( + text: session.remainingText, + geometry: CotabbyTestFixtures.overlayGeometry(caretRect: context.caretRect), + mode: .inline + ) + let inserter = StubSuggestionInserter() + let coordinator = makeCoordinator( + snapshot: snapshot, + overlayState: overlayState, + inputMonitor: StubSuggestionInputMonitor(), + inserter: inserter, + interactionState: interactionState, + settingsSnapshot: CotabbyTestFixtures.settingsSnapshot(addSpaceAfterAccept: true) + ) + coordinator.state = .debouncing + + XCTAssertTrue(coordinator.acceptCurrentSuggestion()) + + // " world" plus the model's own following space, consumed in one accept. + XCTAssertEqual(inserter.insertedChunks, [" world "]) + if case let .ready(remainingText, _) = coordinator.state { + XCTAssertEqual(remainingText, "how", "The consumed following space should not lead the tail.") + } else { + XCTFail("Partial acceptance should leave the remaining suggestion ready.") + } + Self.retainedCoordinators.append(coordinator) + } + } + + func test_acceptCurrentSuggestion_withoutAddSpaceAfterAccept_insertsWordWithoutTrailingSpace() { + // Same setup with the setting OFF: byte-for-byte the prior behavior (no trailing space, the + // following space leads the next chunk). + runOnMainActor { + let snapshot = CotabbyTestFixtures.focusedInputSnapshot(precedingText: "Hello") + let context = FocusedInputContext(snapshot: snapshot, generation: 7) + let interactionState = SuggestionInteractionState() + let session = interactionState.startSession( + fullText: " world how", + liveContext: context, + latency: 0.1 + ) + let overlayState = OverlayState.visible( + text: session.remainingText, + geometry: CotabbyTestFixtures.overlayGeometry(caretRect: context.caretRect), + mode: .inline + ) + let inserter = StubSuggestionInserter() + let coordinator = makeCoordinator( + snapshot: snapshot, + overlayState: overlayState, + inputMonitor: StubSuggestionInputMonitor(), + inserter: inserter, + interactionState: interactionState, + settingsSnapshot: CotabbyTestFixtures.settingsSnapshot(addSpaceAfterAccept: false) + ) + coordinator.state = .debouncing + + XCTAssertTrue(coordinator.acceptCurrentSuggestion()) + + XCTAssertEqual(inserter.insertedChunks, [" world"]) + if case let .ready(remainingText, _) = coordinator.state { + XCTAssertEqual(remainingText, " how") + } else { + XCTFail("Partial acceptance should leave the remaining suggestion ready.") + } + Self.retainedCoordinators.append(coordinator) + } + } + func test_acceptCurrentSuggestionCleansVisibleOverlayWhenSessionDisappears() { runOnMainActor { let snapshot = CotabbyTestFixtures.focusedInputSnapshot(precedingText: "Hello") @@ -381,7 +463,8 @@ final class SuggestionCoordinatorAcceptanceTests: XCTestCase { overlayState: OverlayState, inputMonitor: StubSuggestionInputMonitor, inserter: StubSuggestionInserter, - interactionState: SuggestionInteractionState + interactionState: SuggestionInteractionState, + settingsSnapshot: SuggestionSettingsSnapshot = CotabbyTestFixtures.settingsSnapshot() ) -> SuggestionCoordinator { let focusSnapshot = FocusSnapshot( applicationName: snapshot.applicationName, @@ -390,6 +473,8 @@ final class SuggestionCoordinatorAcceptanceTests: XCTestCase { context: snapshot, inspection: nil ) + let settingsProvider = StubSuggestionSettingsProvider() + settingsProvider.snapshot = settingsSnapshot let coordinator = SuggestionCoordinator( permissionManager: StubSuggestionPermissionProvider(), focusModel: StubSuggestionFocusProvider(snapshot: focusSnapshot), @@ -397,7 +482,7 @@ final class SuggestionCoordinatorAcceptanceTests: XCTestCase { overlayController: StubSuggestionOverlayController(state: overlayState), suggestionInserter: inserter, suggestionEngine: StubSuggestionEngine(), - suggestionSettings: StubSuggestionSettingsProvider(), + suggestionSettings: settingsProvider, clipboardContextProvider: StubClipboardContextProvider(), clipboardRelevanceFilter: StubClipboardRelevanceFilter(), visualContextCoordinator: StubVisualContextCoordinator(),