diff --git a/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Acceptance.swift b/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Acceptance.swift index 6e70ee3a..c35159cd 100644 --- a/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Acceptance.swift +++ b/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Acceptance.swift @@ -83,13 +83,23 @@ extension SuggestionCoordinator { // partial modes (`.word`, `.phrase`), since whole-suggestion acceptance is exclusively the // dedicated full-accept key's job. let primaryGranularity = settingsSnapshot.acceptanceGranularity + // A presentation the overlay is still holding (pixel caret read, lagging host caret) leaves + // `overlayState` naming the pre-accept tail for a few tens of milliseconds. Hand the held + // text to validation so a rapid second Tab accepts the tail Cotabby is about to paint + // instead of mismatching, tearing the session down, and leaking Tab into the host. + let heldPresentationText = overlayController.heldPresentationText let preparation: SuggestionAcceptancePreparation if fullText { - preparation = interactionState.prepareFullAcceptance(from: rawContext, overlayState: overlayState) + preparation = interactionState.prepareFullAcceptance( + from: rawContext, + overlayState: overlayState, + heldPresentationText: heldPresentationText + ) } else { preparation = interactionState.prepareAcceptance( from: rawContext, overlayState: overlayState, + heldPresentationText: heldPresentationText, granularity: primaryGranularity, autoAcceptTrailingPunctuation: settingsSnapshot.autoAcceptTrailingPunctuation ) diff --git a/Cotabby/Models/Suggestion/SuggestionSubsystemContracts.swift b/Cotabby/Models/Suggestion/SuggestionSubsystemContracts.swift index c5fce9a4..9c88d684 100644 --- a/Cotabby/Models/Suggestion/SuggestionSubsystemContracts.swift +++ b/Cotabby/Models/Suggestion/SuggestionSubsystemContracts.swift @@ -243,6 +243,13 @@ protocol SuggestionOverlayControlling: AnyObject { var state: OverlayState { get } var onStateChange: ((OverlayState) -> Void)? { get set } + /// Text the last `showSuggestion` call asked for but that the controller is still holding off + /// screen (waiting for a pixel caret read, or for a host caret that lags its published text). + /// `state` is not updated until the held present lands, so it keeps describing the previous + /// presentation; acceptance consults this to tell "our own present is in flight" apart from a + /// genuinely stale ghost. Nil whenever nothing is held. + var heldPresentationText: String? { get } + func showSuggestion(_ text: String, geometry: SuggestionOverlayGeometry) func hide(reason: String) @@ -260,6 +267,9 @@ protocol SuggestionOverlayControlling: AnyObject { } extension SuggestionOverlayControlling { + /// Default: presentations are applied synchronously, so nothing is ever held. + var heldPresentationText: String? { nil } + /// Default: not supported, so conformers that do not render an inline panel (e.g. test doubles) /// transparently fall back to the caret-anchored present path. func advanceInline(to remainingText: String, insertedText: String) -> Bool { false } diff --git a/Cotabby/Services/Presentation/OverlayController.swift b/Cotabby/Services/Presentation/OverlayController.swift index dfcad5d4..59f5a3db 100644 --- a/Cotabby/Services/Presentation/OverlayController.swift +++ b/Cotabby/Services/Presentation/OverlayController.swift @@ -89,6 +89,12 @@ final class OverlayController: SuggestionOverlayControlling { /// held presentation puts it back. A typed-through advance meanwhile cannot render into the /// hidden panel and asks for a fresh present instead (`advanceInline`). private var panelHeldForCapture = false + /// The text of a presentation `showSuggestion` accepted but has not applied yet (see + /// `SuggestionOverlayControlling.heldPresentationText`). Both holds below return before `state` + /// is reassigned, so without this the coordinator could not tell its own in-flight present + /// from a stale ghost, and rejected a rapid Tab that followed an accept. Cleared by every show + /// that is applied, by every newer show before it decides to hold, and by `hide`. + private(set) var heldPresentationText: String? /// Measures web hosts' painted baselines; nil where screen capture is unwanted (tests). private let baselineCalibrator: HostBaselineCalibrator? /// The faces an Electron host ships in its bundle, for the pixel match (see the registry). @@ -158,6 +164,7 @@ final class OverlayController: SuggestionOverlayControlling { } pixelCaretShowToken &+= 1 panelHeldForCapture = false + heldPresentationText = nil // A caret the host has not yet moved for text it has already published (see // `CaretLagPolicy`): the next snapshot brings the real one, and a ghost drawn from this one // would sit over the typed text, sized from an empty line's box. @@ -171,6 +178,7 @@ final class OverlayController: SuggestionOverlayControlling { "chars": .stringConvertible(requestedGeometry.lineTextBeforeCaret?.count ?? 0) ] ) + heldPresentationText = text return } // A caret inside a union-run paragraph is placed from the host's pixels (see @@ -272,6 +280,9 @@ final class OverlayController: SuggestionOverlayControlling { ) } let token = pixelCaretShowToken + // Marked before `locate`, not after it returns: a capture that fails or answers at once + // calls back synchronously, and that nested `showSuggestion` must be free to clear the mark. + heldPresentationText = text pixelCaretLocator.locate(request) { [weak self] _ in guard let self, self.pixelCaretShowToken == token else { return } self.showSuggestion(text, geometry: requestedGeometry) @@ -283,6 +294,7 @@ final class OverlayController: SuggestionOverlayControlling { func hide(reason: String) { pixelCaretShowToken &+= 1 panelHeldForCapture = false + heldPresentationText = nil CotabbyLogger.suggestion.debug("Overlay hidden", metadata: ["stage": .string("overlay-hide"), "reason": .string(reason)]) panel.orderOut(nil) inlineSession = nil diff --git a/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift b/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift index 86f87fd0..a8360780 100644 --- a/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift +++ b/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift @@ -137,13 +137,20 @@ final class SuggestionInteractionState { /// `granularity` selects between word-by-word and phrase-by-phrase acceptance. Whole- /// suggestion acceptance is the dedicated full-accept key's responsibility and is routed /// through `prepareFullAcceptance`, so the granularity enum has no case for it here. + /// + /// `heldPresentationText` is the overlay controller's not-yet-painted presentation, if any + /// (see `SuggestionSessionReconciler.overlayAllowsAcceptance`). Defaulted so callers that have + /// no deferred presenter keep the strict "visible text must equal the tail" rule. func prepareAcceptance( from snapshot: FocusedInputSnapshot, overlayState: OverlayState, + heldPresentationText: String? = nil, granularity: AcceptanceGranularity, autoAcceptTrailingPunctuation: Bool = true ) -> SuggestionAcceptancePreparation { - let validated = validateSessionForAcceptance(from: snapshot, overlayState: overlayState) + let validated = validateSessionForAcceptance( + from: snapshot, overlayState: overlayState, heldPresentationText: heldPresentationText + ) guard let (liveContext, session) = validated.session else { return .invalid(validated.failureReason ?? "Key passed through.") } @@ -170,9 +177,12 @@ final class SuggestionInteractionState { func prepareFullAcceptance( from snapshot: FocusedInputSnapshot, - overlayState: OverlayState + overlayState: OverlayState, + heldPresentationText: String? = nil ) -> SuggestionAcceptancePreparation { - let validated = validateSessionForAcceptance(from: snapshot, overlayState: overlayState) + let validated = validateSessionForAcceptance( + from: snapshot, overlayState: overlayState, heldPresentationText: heldPresentationText + ) guard let (liveContext, session) = validated.session else { return .invalid(validated.failureReason ?? "Key passed through.") } @@ -192,7 +202,8 @@ final class SuggestionInteractionState { private func validateSessionForAcceptance( from snapshot: FocusedInputSnapshot, - overlayState: OverlayState + overlayState: OverlayState, + heldPresentationText: String? ) -> SessionValidation { guard let activeSession else { return SessionValidation(session: nil, failureReason: "Key passed through because no valid suggestion was ready.") @@ -204,7 +215,8 @@ final class SuggestionInteractionState { guard SuggestionSessionReconciler.overlayAllowsAcceptance( of: activeSession.remainingText, - overlayState: overlayState + overlayState: overlayState, + heldPresentationText: heldPresentationText ) else { return SessionValidation( session: nil, diff --git a/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift b/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift index d2b69f86..ebbd8254 100644 --- a/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift +++ b/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift @@ -651,12 +651,27 @@ enum SuggestionSessionReconciler { /// The overlay may be hidden briefly while waiting for the host app to publish an updated /// caret position, so hidden does not automatically mean "reject Tab." - static func overlayAllowsAcceptance(of text: String, overlayState: OverlayState) -> Bool { + /// + /// `heldPresentationText` is the text the overlay controller was last asked to show but is + /// still holding off screen (a pixel caret read in flight, or a caret that lags the host's + /// published text; see `SuggestionOverlayControlling.heldPresentationText`). While a present is + /// held, `overlayState` still describes the *previous* presentation, so after a Tab accept it + /// names the tail as it was before that accept. A rapid follow-up Tab then compared the new + /// tail with the old one, failed, and passed through: the session was torn down and the host + /// received a real Tab, moving focus to the page's next control. The held text is the offer + /// Cotabby itself is committed to painting for this exact session, so it authorizes acceptance + /// just as a painted ghost does. Any other mismatch still rejects: a visible ghost that is + /// neither the tail nor its pending replacement is stale UI, not something the user was offered. + static func overlayAllowsAcceptance( + of text: String, + overlayState: OverlayState, + heldPresentationText: String? = nil + ) -> Bool { guard case let .visible(visibleText, _, _) = overlayState else { return true } - return visibleText == text + return visibleText == text || heldPresentationText == text } } diff --git a/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift b/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift index e377f06f..189fbc0e 100644 --- a/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift +++ b/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift @@ -124,6 +124,76 @@ final class SuggestionCoordinatorAcceptanceTests: SuggestionCoordinatorRigTestCa XCTAssertEqual(rig.coordinator.state, .ready(text: " how", latency: 0.05)) } + // MARK: - Rapid successive accepts + + func test_rapidTabsAcceptEachWordWhileTheOverlayIsStillHoldingThePreviousPresent() { + // Regression: the overlay controller can hold a presentation (pixel caret read, lagging + // host caret) and leave `overlayState` on the pre-accept tail. A second Tab arriving in that + // window, before the host has even published the first insertion to AX, used to fail the + // "visible text equals tail" check, tear the session down, and pass Tab to the browser, + // which moved focus to the page's other controls. + let rig = retained(makeCoordinatorRig()) + startVisibleSession(in: rig, fullText: " hello world again") + rig.overlayController.defersPresentations = true + + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion()) + XCTAssertEqual(rig.overlayController.heldPresentationText, " world again") + XCTAssertEqual( + visibleText(of: rig.coordinator.overlayState), + " hello world again", + "The held present leaves the published state on the previous tail" + ) + XCTAssertTrue( + rig.inputMonitor.shouldConsumeAcceptKeyProvider(), + "The accept tap must keep owning Tab while the next present is held" + ) + + // The focus snapshot is untouched: AX has not published " hello" yet. + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion(), "The second rapid Tab must be consumed") + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion(), "So must the third") + + XCTAssertEqual(rig.inserter.insertedChunks, [" hello", " world", " again"]) + XCTAssertFalse( + rig.overlayController.hideReasons.contains { $0.hasPrefix("Key passed through") }, + "No Tab may be handed back to the host during the rapid sequence" + ) + } + + func test_heldPresentLandingKeepsTheRemainingTailAcceptable() { + let rig = retained(makeCoordinatorRig()) + startVisibleSession(in: rig, fullText: " hello world again") + rig.overlayController.defersPresentations = true + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion()) + + rig.overlayController.defersPresentations = false + rig.overlayController.landHeldPresentation() + + XCTAssertNil(rig.overlayController.heldPresentationText) + XCTAssertEqual(visibleText(of: rig.coordinator.overlayState), " world again") + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion()) + XCTAssertEqual(rig.inserter.insertedChunks, [" hello", " world"]) + } + + func test_staleVisibleGhostWithoutAHeldPresentStillPassesTabThrough() { + // The held text widens acceptance only for Cotabby's own in-flight present. A ghost that + // shows something other than the tail, with nothing held, is stale UI and must not accept. + let rig = retained(makeCoordinatorRig()) + startVisibleSession(in: rig, fullText: " hello world") + rig.overlayController.showSuggestion( + " something else", + geometry: CotabbyTestFixtures.overlayGeometry() + ) + + XCTAssertFalse(rig.coordinator.acceptCurrentSuggestion()) + XCTAssertTrue(rig.inserter.insertedChunks.isEmpty) + XCTAssertNil(rig.interactionState.activeSession) + } + + private func visibleText(of state: OverlayState) -> String? { + guard case let .visible(text, _, _) = state else { return nil } + return text + } + // MARK: - Insertion failures func test_failedInsertionReturnsTheKeyAndTearsTheSessionDown() { diff --git a/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift b/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift index ce620d02..6e2adbb5 100644 --- a/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift +++ b/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift @@ -164,6 +164,32 @@ final class SuggestionInteractionStateAcceptanceGuardTests: XCTestCase { XCTAssertEqual(chunk, "world") } + func test_secondTabBeforeAXPublishAcceptsTheHeldTailWhileOverlayShowsThePreviousOne() throws { + // Rapid Tab regression: after the first accept, AX still shows the pre-insertion text and + // the overlay's published state still shows the pre-accept tail, because the controller is + // holding the next present. The held text must authorize the second Tab. + let state = makeState() + let snapshot = CotabbyTestFixtures.focusedInputSnapshot(precedingText: "Hello") + let context = FocusedInputContext(snapshot: snapshot, generation: 1) + let session = state.startSession(fullText: " world again", liveContext: context, latency: 0) + _ = state.commitAcceptedChunk(" world", liveContext: context, session: session) + let stalePublishedOverlay = visibleOverlay(text: " world again", for: snapshot) + + guard case .invalid = state.prepareAcceptance( + from: snapshot, overlayState: stalePublishedOverlay, granularity: .word + ) else { return XCTFail("Without a held present, a mismatched visible ghost is stale UI") } + + guard case let .ready(_, prepared, chunk) = state.prepareAcceptance( + from: snapshot, + overlayState: stalePublishedOverlay, + heldPresentationText: " again", + granularity: .word + ) else { return XCTFail("The held tail must accept before AX publishes the first insertion") } + XCTAssertEqual(prepared.consumedCharacterCount, 6) + XCTAssertEqual(chunk, " again") + XCTAssertTrue(state.isAwaitingPostInsertionSync, "The first insertion's publication is still owed") + } + func test_matchingTypingAfterTabPreservesOutstandingInsertionPublication() throws { let state = makeState() let snapshot = CotabbyTestFixtures.focusedInputSnapshot(precedingText: "Hello") diff --git a/CotabbyTests/Support/Suggestion/Acceptance/SuggestionOverlayAcceptanceTests.swift b/CotabbyTests/Support/Suggestion/Acceptance/SuggestionOverlayAcceptanceTests.swift index 737e1ecf..c4238541 100644 --- a/CotabbyTests/Support/Suggestion/Acceptance/SuggestionOverlayAcceptanceTests.swift +++ b/CotabbyTests/Support/Suggestion/Acceptance/SuggestionOverlayAcceptanceTests.swift @@ -38,6 +38,33 @@ final class SuggestionOverlayAcceptanceTests: XCTestCase { ) } + func test_overlayAllowsAcceptance_heldPresentationAuthorizesItsOwnTextOnly() { + // The controller is holding the next present, so the published state still shows the + // tail from before the last accept. + let previousTail = OverlayState.visible( + text: " hello world", + geometry: CotabbyTestFixtures.overlayGeometry(), + mode: .inline + ) + + XCTAssertTrue( + SuggestionSessionReconciler.overlayAllowsAcceptance( + of: " world", overlayState: previousTail, heldPresentationText: " world" + ) + ) + XCTAssertFalse( + SuggestionSessionReconciler.overlayAllowsAcceptance( + of: " world", overlayState: previousTail, heldPresentationText: " there" + ), + "A held present for different text does not make a mismatched ghost acceptable" + ) + XCTAssertFalse( + SuggestionSessionReconciler.overlayAllowsAcceptance( + of: " world", overlayState: previousTail, heldPresentationText: nil + ) + ) + } + func test_overlayHideReason_mapsSemanticInputEventsToUserVisibleReasons() { XCTAssertEqual( SuggestionSessionReconciler.overlayHideReason( diff --git a/CotabbyTests/TestSupport/SuggestionCoordinatorTestSupport.swift b/CotabbyTests/TestSupport/SuggestionCoordinatorTestSupport.swift index 37ed8d35..ed385a0c 100644 --- a/CotabbyTests/TestSupport/SuggestionCoordinatorTestSupport.swift +++ b/CotabbyTests/TestSupport/SuggestionCoordinatorTestSupport.swift @@ -101,6 +101,12 @@ final class RigOverlayController: SuggestionOverlayControlling { /// Records slide attempts (and declines them, like the protocol default) so tests can assert /// which accept paths even try to slide versus re-anchor through a present. private(set) var advanceInlineCalls: [(remaining: String, inserted: String)] = [] + /// When true, `showSuggestion` behaves like the real controller waiting on a pixel caret read + /// or a lagging host caret: it records the text as held and leaves `state` on the previous + /// presentation. `landHeldPresentation()` then applies it, as the capture callback would. + var defersPresentations = false + private(set) var heldPresentationText: String? + private var heldGeometry: SuggestionOverlayGeometry? init(state: OverlayState = .hidden(reason: "initial")) { self.state = state @@ -113,12 +119,29 @@ final class RigOverlayController: SuggestionOverlayControlling { func showSuggestion(_ text: String, geometry: SuggestionOverlayGeometry) { shownTexts.append(text) + if defersPresentations { + heldPresentationText = text + heldGeometry = geometry + return + } + heldPresentationText = nil + state = .visible(text: text, geometry: geometry, mode: .inline) + onStateChange?(state) + } + + /// Applies the held presentation, the way the real controller's capture callback re-runs it. + func landHeldPresentation() { + guard let text = heldPresentationText, let geometry = heldGeometry else { return } + heldPresentationText = nil + heldGeometry = nil state = .visible(text: text, geometry: geometry, mode: .inline) onStateChange?(state) } func hide(reason: String) { hideReasons.append(reason) + heldPresentationText = nil + heldGeometry = nil state = .hidden(reason: reason) onStateChange?(state) }