diff --git a/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Input.swift b/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Input.swift index c0730e75..5fc15e7c 100644 --- a/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Input.swift +++ b/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Input.swift @@ -454,9 +454,9 @@ extension SuggestionCoordinator { // round-trip the speculation existed to skip. Stand down and let it land; `apply` // validates via the same signature. Any divergence falls through to the normal // reschedule, whose newer work id retires the speculation automatically. - if let expected = pendingSpeculativeContext, - currentContext?.sessionIdentity == expected.sessionIdentity, - currentContext?.contentSignature == expected.contentSignature { + if let expected = pendingSpeculativeContext, let currentContext, + currentContext.sessionIdentity.continues(expected.sessionIdentity), + currentContext.contentSignature == expected.contentSignature { logStage( "speculation-validated", workID: currentWorkID, diff --git a/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Prediction.swift b/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Prediction.swift index 3df7bcc2..63c087a2 100644 --- a/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Prediction.swift +++ b/Cotabby/App/Coordinators/Suggestion/SuggestionCoordinator+Prediction.swift @@ -863,9 +863,10 @@ extension SuggestionCoordinator { // not published yet, so its generation predates the live one by construction. When the // live content now matches the signature the speculation was built against, the bet paid // off and the result is exactly current. - let isPaidOffSpeculation = pendingSpeculativeContext != nil - && pendingSpeculativeContext?.sessionIdentity == liveContext.sessionIdentity - && pendingSpeculativeContext?.contentSignature == liveContext.contentSignature + let isPaidOffSpeculation = pendingSpeculativeContext.map { speculated in + liveContext.sessionIdentity.continues(speculated.sessionIdentity) + && liveContext.contentSignature == speculated.contentSignature + } ?? false if isPaidOffSpeculation { pendingSpeculativeContext = nil } diff --git a/Cotabby/Models/Focus/FocusModels.swift b/Cotabby/Models/Focus/FocusModels.swift index 0ac67ebf..a937e64a 100644 --- a/Cotabby/Models/Focus/FocusModels.swift +++ b/Cotabby/Models/Focus/FocusModels.swift @@ -26,6 +26,34 @@ nonisolated struct FocusedInputSessionIdentity: Hashable, Sendable { let focusedURLString: String? let windowTitle: String? let fieldPlaceholder: String? + + /// True when this identity, read from a live poll, still describes the writing session + /// `previous` was captured in. + /// + /// This is the comparison every consumer of a *live* snapshot must use instead of `==`. The + /// process, bundle and focus sequence are Cotabby's own bookkeeping and always known, so they + /// must match exactly. The three surface facts are bounded AX reads repeated on every poll, + /// each under `AXHelper`'s 50 ms messaging timeout. A host busy processing a burst of synthetic + /// keystrokes (Chromium during rapid Tab accepts) times one out, and the fact arrives as nil. + /// Nil is an unreadable fact, not a navigation: navigation is one known value replaced by a + /// different known value. Reading nil as "changed" tore the active suggestion down mid-burst as + /// a "focused field" change, and the next Tab reached the page as a real Tab. + func continues(_ previous: FocusedInputSessionIdentity) -> Bool { + processIdentifier == previous.processIdentifier + && bundleIdentifier == previous.bundleIdentifier + && focusChangeSequence == previous.focusChangeSequence + && Self.surfaceFactsAgree(focusedURLString, previous.focusedURLString) + && Self.surfaceFactsAgree(windowTitle, previous.windowTitle) + && Self.surfaceFactsAgree(fieldPlaceholder, previous.fieldPlaceholder) + } + + /// Two reads of one surface fact agree unless both are known and differ. Shared with + /// `FocusedInputPollingSignature` so the tracker's navigation signal and the session identity + /// consumers rely on cannot drift apart on what an unreadable fact means. + static func surfaceFactsAgree(_ lhs: String?, _ rhs: String?) -> Bool { + guard let lhs, let rhs else { return true } + return lhs == rhs + } } /// Describes how trustworthy the resolved caret rect is. diff --git a/Cotabby/Services/Focus/FocusTracker.swift b/Cotabby/Services/Focus/FocusTracker.swift index 3a5f3058..c0c560b1 100644 --- a/Cotabby/Services/Focus/FocusTracker.swift +++ b/Cotabby/Services/Focus/FocusTracker.swift @@ -374,8 +374,11 @@ final class FocusTracker { let nextSignature = FocusedInputPollingSignature(context: context) if let lastFocusedInputSignature, nextSignature.continuesField(of: lastFocusedInputSignature) { // Same field, possibly resized in place. Track its latest frame so later growth is - // compared with the current edges, without opening a new writing session. - self.lastFocusedInputSignature = nextSignature + // compared with the current edges, without opening a new writing session. A surface + // fact this poll failed to read (a title or URL query that hit the AX timeout while the + // host was busy) keeps its last known value, so a later genuinely different value is + // still recognized as navigation rather than compared against a blank. + self.lastFocusedInputSignature = nextSignature.carryingKnownSurfaceFacts(from: lastFocusedInputSignature) return FocusCaptureResult(snapshot: firstPassSnapshot, didChangeFocusedInput: false) } diff --git a/Cotabby/Services/Suggestion/State/ContextBuffer.swift b/Cotabby/Services/Suggestion/State/ContextBuffer.swift index 1f02bbe3..caa1ad46 100644 --- a/Cotabby/Services/Suggestion/State/ContextBuffer.swift +++ b/Cotabby/Services/Suggestion/State/ContextBuffer.swift @@ -20,7 +20,11 @@ final class ContextBuffer { // Identical drafts in two chat tabs are different requests. Session identity includes // navigation but excludes volatile AX tokens, so a wrapper refresh alone stays harmless. - if snapshot.sessionIdentity != lastSessionIdentity || signature != lastSignature { + // A surface fact one poll failed to read is not navigation either: bumping the generation + // for it would retire an in-flight suggestion for text that never changed. Real navigation + // still bumps, because FocusTracker advances the focus sequence for it. + let continuesSession = lastSessionIdentity.map { snapshot.sessionIdentity.continues($0) } ?? false + if !continuesSession || signature != lastSignature { nextGeneration &+= 1 } diff --git a/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift b/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift index a8360780..32749c39 100644 --- a/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift +++ b/Cotabby/Services/Suggestion/State/SuggestionInteractionState.swift @@ -96,7 +96,7 @@ final class SuggestionInteractionState { return false } - return currentContext.sessionIdentity != focusedContext.sessionIdentity + return !focusedContext.sessionIdentity.continues(currentContext.sessionIdentity) } /// Reconciles the currently active session against the latest AX snapshot and stores the @@ -244,7 +244,7 @@ final class SuggestionInteractionState { sessionForAcceptance = reconciledSession } } else { - guard liveContext.sessionIdentity == activeSession.baseContext.sessionIdentity else { + guard liveContext.sessionIdentity.continues(activeSession.baseContext.sessionIdentity) else { return SessionValidation(session: nil, failureReason: "Key passed through because the focused field changed.") } diff --git a/Cotabby/Services/Visual/VisualContextCoordinator.swift b/Cotabby/Services/Visual/VisualContextCoordinator.swift index a5d09c5a..487282df 100644 --- a/Cotabby/Services/Visual/VisualContextCoordinator.swift +++ b/Cotabby/Services/Visual/VisualContextCoordinator.swift @@ -77,7 +77,7 @@ final class VisualContextCoordinator { // Surface facts can change even when an app reuses its composer and AX handle. Drop the // old excerpt synchronously; the settle delay below must never expose another chat's text. if let previous = activeSessionIdentity ?? pendingStartContext?.sessionIdentity, - previous != snapshotContext.sessionIdentity { + !snapshotContext.sessionIdentity.continues(previous) { cancel(resetState: true) } // Coalesce repeated calls for the same field (active or already pending) so a flapping focus @@ -172,7 +172,7 @@ final class VisualContextCoordinator { guard !Task.isCancelled, activeAugmentationSession?.sessionID == session.sessionID else { return } guard screenRecordingPermissionProvider(), let currentContext, currentContext.identity == snapshotContext.identity, - currentContext.sessionIdentity == snapshotContext.sessionIdentity, !currentContext.isSecure else { + currentContext.sessionIdentity.continues(snapshotContext.sessionIdentity), !currentContext.isSecure else { cancel(resetState: true) return } @@ -205,9 +205,10 @@ final class VisualContextCoordinator { if let provider = refreshContextProvider { let liveContext = provider() guard activeAugmentationSession?.sessionID == session.sessionID else { return } - guard screenRecordingPermissionProvider(), liveContext?.identity == snapshotContext.identity, - liveContext?.sessionIdentity == snapshotContext.sessionIdentity, - liveContext?.isSecure == false else { + guard screenRecordingPermissionProvider(), let liveContext, + liveContext.identity == snapshotContext.identity, + liveContext.sessionIdentity.continues(snapshotContext.sessionIdentity), + !liveContext.isSecure else { cancel(resetState: true) return } @@ -253,7 +254,8 @@ final class VisualContextCoordinator { let context = liveContext, context.elementIdentifier == session.elementIdentifier, context.focusChangeSequence == session.focusChangeSequence, - context.sessionIdentity == self.activeSessionIdentity, + let activeSessionIdentity = self.activeSessionIdentity, + context.sessionIdentity.continues(activeSessionIdentity), !context.isSecure else { self.cancel(resetState: true) return @@ -291,6 +293,12 @@ final class VisualContextCoordinator { /// visual-context session still belongs to that same field. func excerpt(for context: FocusedInputContext) -> String? { expireExcerptIfNeeded() + // Exact identity here, unlike the session-keeping checks above. Those tolerate a surface + // fact the poll failed to read so a busy host does not cancel the capture; this call hands + // screen text to a prompt, and a chat switch that reuses the composer, frame and URL while + // its new title is momentarily unreadable would otherwise condition the new chat's request + // on the previous chat's excerpt. Withholding the excerpt for that one poll costs a request + // its screen context; the session and its capture survive for the next readable poll. guard let activeAugmentationSession, activeSessionIdentity == context.sessionIdentity, activeAugmentationSession.elementIdentifier == context.elementIdentifier, diff --git a/Cotabby/Support/Focus/FocusedInputPollingSignature.swift b/Cotabby/Support/Focus/FocusedInputPollingSignature.swift index ae7bef04..ef23f638 100644 --- a/Cotabby/Support/Focus/FocusedInputPollingSignature.swift +++ b/Cotabby/Support/Focus/FocusedInputPollingSignature.swift @@ -33,18 +33,67 @@ nonisolated struct FocusedInputPollingSignature: Equatable { ) } + private init( + bundleIdentifier: String, + processIdentifier: Int32, + role: String, + subrole: String?, + fieldAnchor: FieldAnchor, + windowTitle: String?, + focusedURLString: String?, + fieldPlaceholder: String? + ) { + self.bundleIdentifier = bundleIdentifier + self.processIdentifier = processIdentifier + self.role = role + self.subrole = subrole + self.fieldAnchor = fieldAnchor + self.windowTitle = windowTitle + self.focusedURLString = focusedURLString + self.fieldPlaceholder = fieldPlaceholder + } + /// True when this poll still observes the field `previous` described. Every identity fact must /// match, but the frame may resize in place: a chat composer grows when a line wraps, keeping /// its left edge, width, and either its top or bottom edge. Treating that as navigation would /// start a new writing session and discard the visible suggestion on every wrap. Two distinct /// fields stacked at the same x and width still differ at both edges. + /// + /// The surface facts (title, URL, placeholder) are compared with the same rule the session + /// identity uses: a fact that read as nil this poll is unreadable, not different. Each is a + /// bounded AX read under a 50 ms timeout, and a host busy with a burst of synthetic keystrokes + /// drops one now and then. Treating that as navigation advanced the focus sequence and retired + /// the active suggestion in the middle of rapid Tab accepts. func continuesField(of previous: FocusedInputPollingSignature) -> Bool { guard bundleIdentifier == previous.bundleIdentifier, processIdentifier == previous.processIdentifier, - role == previous.role, subrole == previous.subrole, windowTitle == previous.windowTitle, - focusedURLString == previous.focusedURLString, fieldPlaceholder == previous.fieldPlaceholder + role == previous.role, subrole == previous.subrole, + FocusedInputSessionIdentity.surfaceFactsAgree(windowTitle, previous.windowTitle), + FocusedInputSessionIdentity.surfaceFactsAgree(focusedURLString, previous.focusedURLString), + FocusedInputSessionIdentity.surfaceFactsAgree(fieldPlaceholder, previous.fieldPlaceholder) else { return false } return fieldAnchor.continues(previous.fieldAnchor) } + + /// This poll's signature with any surface fact it failed to read filled in from `previous`. + /// + /// The tracker stores this, not the raw poll, as the field's latest signature. Otherwise a + /// navigation that straddles one unreadable poll would go unnoticed: title A, then nil (which + /// continues A), then title B (which would continue nil). Carrying A forward makes the B poll + /// compare against A and read as the navigation it is. The frame is always this poll's own, so + /// in-place growth keeps being measured from the current edges. + func carryingKnownSurfaceFacts(from previous: FocusedInputPollingSignature?) -> FocusedInputPollingSignature { + guard let previous else { return self } + return FocusedInputPollingSignature( + bundleIdentifier: bundleIdentifier, + processIdentifier: processIdentifier, + role: role, + subrole: subrole, + fieldAnchor: fieldAnchor, + windowTitle: windowTitle ?? previous.windowTitle, + focusedURLString: focusedURLString ?? previous.focusedURLString, + fieldPlaceholder: fieldPlaceholder ?? previous.fieldPlaceholder + ) + } } private extension FocusedInputPollingSignature { diff --git a/Cotabby/Support/Suggestion/Session/SuggestionContinuationPlan.swift b/Cotabby/Support/Suggestion/Session/SuggestionContinuationPlan.swift index 1a784c49..c0924f80 100644 --- a/Cotabby/Support/Suggestion/Session/SuggestionContinuationPlan.swift +++ b/Cotabby/Support/Suggestion/Session/SuggestionContinuationPlan.swift @@ -106,7 +106,7 @@ nonisolated struct SuggestionContinuationPlan: Equatable, Sendable { /// to web fields with the same real focus sequence and frame; native fields, missing geometry, /// and legacy snapshots without a sequence continue to require the exact element identifier. static func sameFocusedField(_ snapshot: FocusedInputSnapshot, context expected: FocusedInputContext) -> Bool { - guard snapshot.sessionIdentity == expected.sessionIdentity, + guard snapshot.sessionIdentity.continues(expected.sessionIdentity), snapshot.role == expected.role, snapshot.subrole == expected.subrole else { return false } if snapshot.elementIdentifier == expected.elementIdentifier { return true } guard snapshot.isWebContentField, expected.isWebContentField, diff --git a/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift b/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift index ebbd8254..b7bf0e32 100644 --- a/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift +++ b/Cotabby/Support/Suggestion/Session/SuggestionSessionReconciler.swift @@ -61,7 +61,9 @@ enum SuggestionSessionReconciler { // Text may be identical in two conversations. Validate the writing session before even // the post-insertion AX-lag tolerance, which must never authorize a different target. - guard liveContext.sessionIdentity == session.baseContext.sessionIdentity else { + // `continues`, not `==`: a surface fact the live poll failed to read is not a new target + // (see `FocusedInputSessionIdentity.continues`). + guard liveContext.sessionIdentity.continues(session.baseContext.sessionIdentity) else { return .invalid("Overlay hidden because the focused field changed.") } diff --git a/CotabbyTests/App/Coordinators/Suggestion/SuggestionConversationIsolationTests.swift b/CotabbyTests/App/Coordinators/Suggestion/SuggestionConversationIsolationTests.swift index 66777387..9d26bd3e 100644 --- a/CotabbyTests/App/Coordinators/Suggestion/SuggestionConversationIsolationTests.swift +++ b/CotabbyTests/App/Coordinators/Suggestion/SuggestionConversationIsolationTests.swift @@ -38,7 +38,9 @@ final class SuggestionConversationIsolationTests: XCTestCase { } func test_speculativeTextMatchCannotOverrideConversationMismatch() async { - let rig = makeCoordinatorRig() + // The speculation was built while the title was readable; a different known title is + // another conversation (a nil one would only be a timed-out read of the same field). + let rig = makeCoordinatorRig(snapshot: CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "This conversation")) defer { rig.coordinator.stop() } let source = rig.interactionState.materializeContext(from: rig.focusProvider.snapshot.context!) rig.coordinator.pendingSpeculativeContext = source diff --git a/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift b/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift index 189fbc0e..c634f832 100644 --- a/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift +++ b/CotabbyTests/App/Coordinators/Suggestion/SuggestionCoordinatorAcceptanceTests.swift @@ -159,6 +159,70 @@ final class SuggestionCoordinatorAcceptanceTests: SuggestionCoordinatorRigTestCa ) } + /// A browser field whose title and URL were readable when the suggestion was offered. + private var browserComposerSnapshot: FocusedInputSnapshot { + CotabbyTestFixtures.focusedInputSnapshot( + precedingText: "Hello", isWebContentField: true, + focusedURLString: "https://chat.example/one", windowTitle: "Chat one" + ) + } + + private func setLiveSnapshot(_ context: FocusedInputSnapshot, in rig: CoordinatorRig) { + rig.focusProvider.snapshot = FocusSnapshot( + applicationName: context.applicationName, + bundleIdentifier: context.bundleIdentifier, + capability: .supported, + context: context + ) + } + + func test_rapidTabIsStillConsumedWhenThePreAcceptRefreshCannotReadTheTitleOrURL() { + // Regression: the accept path refreshes the focus snapshot synchronously, inside the event + // tap, while the host is still busy with the previous synthetic insertion. The window title + // and page URL are re-read on that refresh under a 50 ms AX timeout, and a busy Chromium + // answers one of them with nothing. That nil used to fail the session-identity equality as + // a "focused field" change: the session was torn down and Tab reached the page, moving + // focus to its buttons. Slow Tabs never hit it because the host was idle by the next read. + let rig = retained(makeCoordinatorRig(snapshot: browserComposerSnapshot)) + startVisibleSession(in: rig, fullText: " hello world again") + + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion()) + + // The second Tab's refresh reads the same field, but its title and URL time out. AX has not + // published " hello" yet either. + setLiveSnapshot(CotabbyTestFixtures.focusedInputSnapshot( + precedingText: "Hello", isWebContentField: true, focusedURLString: nil, windowTitle: nil + ), in: rig) + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion(), "An unreadable title is not another field") + + // The third Tab's refresh reads both facts again. + setLiveSnapshot(browserComposerSnapshot, in: rig) + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion()) + + XCTAssertEqual(rig.inserter.insertedChunks, [" hello", " world", " again"]) + XCTAssertFalse( + rig.overlayController.hideReasons.contains { $0.contains("focused field changed") }, + "A timed-out surface read must not be mistaken for navigation" + ) + } + + func test_tabStillPassesThroughWhenTheRefreshReadsAnotherConversation() { + // The tolerance is for facts that could not be read, never for facts that read differently: + // identical draft text in another conversation must still hand Tab back. + let rig = retained(makeCoordinatorRig(snapshot: browserComposerSnapshot)) + startVisibleSession(in: rig, fullText: " hello world") + XCTAssertTrue(rig.coordinator.acceptCurrentSuggestion()) + + setLiveSnapshot(CotabbyTestFixtures.focusedInputSnapshot( + precedingText: "Hello", isWebContentField: true, + focusedURLString: "https://chat.example/two", windowTitle: "Chat two" + ), in: rig) + + XCTAssertFalse(rig.coordinator.acceptCurrentSuggestion()) + XCTAssertEqual(rig.inserter.insertedChunks, [" hello"]) + XCTAssertNil(rig.interactionState.activeSession) + } + func test_heldPresentLandingKeepsTheRemainingTailAcceptable() { let rig = retained(makeCoordinatorRig()) startVisibleSession(in: rig, fullText: " hello world again") diff --git a/CotabbyTests/Models/Focus/FocusModelsTests.swift b/CotabbyTests/Models/Focus/FocusModelsTests.swift index bf977e25..c7c26aea 100644 --- a/CotabbyTests/Models/Focus/FocusModelsTests.swift +++ b/CotabbyTests/Models/Focus/FocusModelsTests.swift @@ -91,6 +91,47 @@ final class FocusModelsTests: XCTestCase { XCTAssertEqual(snapshot.identity, FocusedInputIdentity(elementIdentifier: "field-a", focusChangeSequence: 4)) } + /// The surface facts are re-read from the host on every poll under a short AX timeout, so a + /// live identity can carry a nil where the session's base identity has a value. That is an + /// unreadable fact, not a navigation; only two known, different values are. + func test_sessionIdentity_continuesAcrossUnreadableSurfaceFactsButNotAcrossChangedOnes() { + let base = CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: "Chat one", fieldPlaceholder: "Message" + ).sessionIdentity + + let unreadable: [(String, FocusedInputSnapshot)] = [ + ("title timed out", CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: nil, fieldPlaceholder: "Message")), + ("url timed out", CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: nil, windowTitle: "Chat one", fieldPlaceholder: "Message")), + ("every fact timed out", CotabbyTestFixtures.focusedInputSnapshot()) + ] + for (label, live) in unreadable { + XCTAssertTrue(live.sessionIdentity.continues(base), label) + XCTAssertNotEqual(live.sessionIdentity, base, "\(label): equality is deliberately stricter") + } + // A base captured during an unreadable poll must also accept the later known value. + XCTAssertTrue(base.continues(CotabbyTestFixtures.focusedInputSnapshot().sessionIdentity)) + + let navigated: [(String, FocusedInputSnapshot)] = [ + ("new title", CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: "Chat two", fieldPlaceholder: "Message")), + ("new url", CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/two", windowTitle: "Chat one", fieldPlaceholder: "Message")), + ("new placeholder", CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: "Chat one", fieldPlaceholder: "Reply")), + ("new focus sequence", CotabbyTestFixtures.focusedInputSnapshot( + focusChangeSequence: 2, focusedURLString: "https://chat.example/one", + windowTitle: "Chat one", fieldPlaceholder: "Message")), + ("other process", CotabbyTestFixtures.focusedInputSnapshot( + processIdentifier: 456, focusedURLString: "https://chat.example/one", + windowTitle: "Chat one", fieldPlaceholder: "Message")) + ] + for (label, live) in navigated { + XCTAssertFalse(live.sessionIdentity.continues(base), label) + } + } + func test_focusedInputSnapshot_flagsPossibleTruncationAtTheCaptureWindow() { let window = FocusedInputSnapshot.textWindowUTF16 let underWindow = String(repeating: "a", count: window - 1) diff --git a/CotabbyTests/Services/Suggestion/State/ContextBufferNavigationTests.swift b/CotabbyTests/Services/Suggestion/State/ContextBufferNavigationTests.swift index bd765abf..9d51ed8d 100644 --- a/CotabbyTests/Services/Suggestion/State/ContextBufferNavigationTests.swift +++ b/CotabbyTests/Services/Suggestion/State/ContextBufferNavigationTests.swift @@ -23,11 +23,22 @@ final class ContextBufferNavigationTests: XCTestCase { func test_surfaceChangeRejectsStaleGenerationEvenBeforeSequenceChanges() { let buffer = makeBuffer() - let first = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot()) + let first = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "This chat")) let navigated = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "Other chat")) XCTAssertGreaterThan(navigated.generation, first.generation) } + /// A poll whose title read timed out is the same session: bumping the generation for it would + /// retire an in-flight suggestion for text that never changed. + func test_unreadableSurfaceFactKeepsGeneration() { + let buffer = makeBuffer() + let first = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "This chat")) + let blankRead = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot()) + let readable = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "This chat")) + XCTAssertEqual(first.generation, blankRead.generation) + XCTAssertEqual(first.generation, readable.generation) + } + func test_wrapperChurnAloneKeepsGeneration() { let buffer = makeBuffer() let first = buffer.materialize(from: CotabbyTestFixtures.focusedInputSnapshot()) diff --git a/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift b/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift index 6e2adbb5..1c70d4bf 100644 --- a/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift +++ b/CotabbyTests/Services/Suggestion/State/SuggestionInteractionStateTests.swift @@ -355,10 +355,12 @@ final class SuggestionInteractionStateAcceptanceGuardTests: XCTestCase { func test_hasFocusedElementChanged_tracksSessionIdentityNotAXWrapperChurn() { let state = makeState() - _ = state.materializeContext(from: CotabbyTestFixtures.focusedInputSnapshot()) + // The field's title was readable when the context was materialized, so a *different* title + // later is a conversation switch (a nil one would only be an unreadable poll). + _ = state.materializeContext(from: CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "This chat")) let cases: [(FocusedInputSnapshot, Bool, String)] = [ - (CotabbyTestFixtures.focusedInputSnapshot(), false, "same field"), + (CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "This chat"), false, "same field"), (CotabbyTestFixtures.focusedInputSnapshot(elementIdentifier: "new-wrapper"), false, "wrapper refresh"), (CotabbyTestFixtures.focusedInputSnapshot(focusChangeSequence: 2), true, "real focus change"), (CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "Other chat"), true, "reused composer, new chat") @@ -368,6 +370,24 @@ final class SuggestionInteractionStateAcceptanceGuardTests: XCTestCase { } } + /// A poll whose title or URL read timed out (nil) is the same field; only a different known + /// value is a conversation switch. Reading nil as a switch tore the session down mid-accept. + func test_hasFocusedElementChanged_ignoresSurfaceFactsThatFailedToRead() { + let state = makeState() + _ = state.materializeContext(from: CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: "Chat one" + )) + + XCTAssertFalse(state.hasFocusedElementChanged(comparedTo: CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: nil + )), "title read timed out") + XCTAssertFalse(state.hasFocusedElementChanged(comparedTo: CotabbyTestFixtures.focusedInputSnapshot()), + "every surface read timed out") + XCTAssertTrue(state.hasFocusedElementChanged(comparedTo: CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/one", windowTitle: "Chat two" + )), "a different known title is still a switch") + } + /// A session started from a context that never went through the buffer still anchors the /// comparison, so a stale session cannot be kept alive just because no snapshot was buffered. func test_hasFocusedElementChanged_fallsBackToTheSessionBaseContext() { diff --git a/CotabbyTests/Support/Focus/FocusedInputPollingSignatureTests.swift b/CotabbyTests/Support/Focus/FocusedInputPollingSignatureTests.swift index 043ef16d..c77fbd08 100644 --- a/CotabbyTests/Support/Focus/FocusedInputPollingSignatureTests.swift +++ b/CotabbyTests/Support/Focus/FocusedInputPollingSignatureTests.swift @@ -46,16 +46,33 @@ final class FocusedInputPollingSignatureTests: XCTestCase { /// Every identity fact must match for continuity, even when the composer geometry is reused /// verbatim (a chat app swapping conversations inside one window is the motivating case). + /// The original's surface facts are known values: a fact that changes from one known value to + /// another is navigation, whereas a fact that merely failed to read is covered separately below. func test_identityFactsDistinguishReusedComposerAndBreakContinuity() { - let original = FocusedInputPollingSignature(context: CotabbyTestFixtures.focusedInputSnapshot()) + let url = "https://chat.example/conversation/one" + let title = "Conversation one" + let placeholder = "Message #channel" + let original = FocusedInputPollingSignature(context: CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: url, windowTitle: title, fieldPlaceholder: placeholder + )) + func snapshot( + processIdentifier: Int32 = 123, bundleIdentifier: String = "com.example.TestApp", + role: String = "AXTextField", subrole: String? = nil, + focusedURLString: String = url, windowTitle: String = title, fieldPlaceholder: String = placeholder + ) -> FocusedInputSnapshot { + CotabbyTestFixtures.focusedInputSnapshot( + bundleIdentifier: bundleIdentifier, processIdentifier: processIdentifier, role: role, subrole: subrole, + focusedURLString: focusedURLString, windowTitle: windowTitle, fieldPlaceholder: fieldPlaceholder + ) + } let changes: [(label: String, snapshot: FocusedInputSnapshot)] = [ - ("url", CotabbyTestFixtures.focusedInputSnapshot(focusedURLString: "https://chat.example/conversation/two")), - ("window title", CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "Another conversation")), - ("placeholder", CotabbyTestFixtures.focusedInputSnapshot(fieldPlaceholder: "Message #another-channel")), - ("pid", CotabbyTestFixtures.focusedInputSnapshot(processIdentifier: 456)), - ("bundle", CotabbyTestFixtures.focusedInputSnapshot(bundleIdentifier: "com.example.Other")), - ("role", CotabbyTestFixtures.focusedInputSnapshot(role: "AXTextArea")), - ("subrole", CotabbyTestFixtures.focusedInputSnapshot(subrole: "AXSearchField")) + ("url", snapshot(focusedURLString: "https://chat.example/conversation/two")), + ("window title", snapshot(windowTitle: "Another conversation")), + ("placeholder", snapshot(fieldPlaceholder: "Message #another-channel")), + ("pid", snapshot(processIdentifier: 456)), + ("bundle", snapshot(bundleIdentifier: "com.example.Other")), + ("role", snapshot(role: "AXTextArea")), + ("subrole", snapshot(subrole: "AXSearchField")) ] for (label, snapshot) in changes { let changed = FocusedInputPollingSignature(context: snapshot) @@ -64,6 +81,45 @@ final class FocusedInputPollingSignatureTests: XCTestCase { } } + /// A surface fact that fails to read on one poll (the host was busy and the AX call hit its + /// timeout) arrives as nil. That poll still observes the same field: treating it as navigation + /// advanced the focus sequence and retired the active suggestion mid-way through rapid accepts. + func test_unreadableSurfaceFactsContinueTheField() { + let known = FocusedInputPollingSignature(context: CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/conversation/one", windowTitle: "Conversation one", + fieldPlaceholder: "Message" + )) + let blankPolls: [(label: String, snapshot: FocusedInputSnapshot)] = [ + ("title", CotabbyTestFixtures.focusedInputSnapshot( + focusedURLString: "https://chat.example/conversation/one", fieldPlaceholder: "Message")), + ("url", CotabbyTestFixtures.focusedInputSnapshot(windowTitle: "Conversation one", fieldPlaceholder: "Message")), + ("all facts", CotabbyTestFixtures.focusedInputSnapshot()) + ] + for (label, snapshot) in blankPolls { + let blank = FocusedInputPollingSignature(context: snapshot) + XCTAssertTrue(blank.continuesField(of: known), label) + XCTAssertTrue(known.continuesField(of: blank), "\(label): the facts coming back is not navigation either") + } + } + + /// The tracker stores each continuing poll with unreadable facts filled from the previous one, + /// so a navigation that straddles a blank poll (A, nil, B) is still caught on the B poll. + func test_carriedKnownFactsStillCatchNavigationAfterAnUnreadablePoll() { + let first = FocusedInputPollingSignature(context: CotabbyTestFixtures.focusedInputSnapshot( + windowTitle: "Conversation one" + )) + let blank = FocusedInputPollingSignature(context: CotabbyTestFixtures.focusedInputSnapshot()) + let second = FocusedInputPollingSignature(context: CotabbyTestFixtures.focusedInputSnapshot( + windowTitle: "Conversation two" + )) + + XCTAssertTrue(second.continuesField(of: blank), "Against the raw blank poll the switch is invisible") + let carried = blank.carryingKnownSurfaceFacts(from: first) + XCTAssertEqual(carried, first, "Nothing but the unreadable title was inherited") + XCTAssertFalse(second.continuesField(of: carried), "Against the carried signature it is navigation") + XCTAssertEqual(blank.carryingKnownSurfaceFacts(from: nil), blank) + } + func test_subPointFrameJitterRoundsToTheSameSignature() { // AX frames arrive as floating-point points; rounding keeps half-point jitter from reading as // a different field. diff --git a/CotabbyTests/Support/Suggestion/Session/SuggestionSessionReconciliationTests.swift b/CotabbyTests/Support/Suggestion/Session/SuggestionSessionReconciliationTests.swift index fed5d92f..fd68410a 100644 --- a/CotabbyTests/Support/Suggestion/Session/SuggestionSessionReconciliationTests.swift +++ b/CotabbyTests/Support/Suggestion/Session/SuggestionSessionReconciliationTests.swift @@ -5,12 +5,23 @@ import XCTest /// consumed-prefix changes advance it, and how the post-insertion and typed-input lag sentinels /// tolerate a host that has not published yet without excusing unrelated edits. final class SuggestionSessionReconciliationTests: XCTestCase { + /// The session was offered in a field whose title and URL were readable; a poll that reads a + /// *different* title or URL (or a new focus sequence) is another conversation, however similar + /// its text. A poll that fails to read one of them is not; see the next test. func test_identicalTextInAnotherConversationRejectsEvenDuringInsertionLag() { - let session = CotabbyTestFixtures.activeSession() + let session = ActiveSuggestionSession( + baseContext: CotabbyTestFixtures.focusedInputContext( + focusedURLString: "https://chat.example/one", windowTitle: "This chat" + ), + fullText: " world again", + consumedCharacterCount: 0, + latency: 0.1 + ) let targets = [ - CotabbyTestFixtures.focusedInputContext(focusChangeSequence: 2), - CotabbyTestFixtures.focusedInputContext(windowTitle: "Other chat"), - CotabbyTestFixtures.focusedInputContext(focusedURLString: "https://chat.example/two") + CotabbyTestFixtures.focusedInputContext( + focusChangeSequence: 2, focusedURLString: "https://chat.example/one", windowTitle: "This chat"), + CotabbyTestFixtures.focusedInputContext(focusedURLString: "https://chat.example/one", windowTitle: "Other chat"), + CotabbyTestFixtures.focusedInputContext(focusedURLString: "https://chat.example/two", windowTitle: "This chat") ] for target in targets { assertInvalid(SuggestionSessionReconciler.reconcile( @@ -19,6 +30,36 @@ final class SuggestionSessionReconciliationTests: XCTestCase { } } + /// The live poll re-reads the title and URL from the host under a short AX timeout; during a + /// burst of synthetic insertions one comes back nil. That is the same field, so the session + /// (here still waiting for the host to publish the first insertion) must survive it. Reading + /// nil as another conversation retired the session and handed the next Tab to the host. + func test_unreadableSurfaceFactsDuringInsertionLagKeepTheSession() { + let session = ActiveSuggestionSession( + baseContext: CotabbyTestFixtures.focusedInputContext( + focusedURLString: "https://chat.example/one", windowTitle: "Chat one" + ), + fullText: " world again", + consumedCharacterCount: 6, + latency: 0.1 + ) + let blankReads = [ + CotabbyTestFixtures.focusedInputContext(focusedURLString: "https://chat.example/one", windowTitle: nil), + CotabbyTestFixtures.focusedInputContext(focusedURLString: nil, windowTitle: "Chat one"), + CotabbyTestFixtures.focusedInputContext() + ] + for live in blankReads { + guard case .valid = SuggestionSessionReconciler.reconcile( + session: session, with: live, pendingInsertionConsumedCount: session.consumedCharacterCount + ) else { return XCTFail("A surface fact that failed to read must not end the session: \(live.sessionIdentity)") } + } + assertInvalid(SuggestionSessionReconciler.reconcile( + session: session, + with: CotabbyTestFixtures.focusedInputContext(focusedURLString: "https://chat.example/one", windowTitle: "Chat two"), + pendingInsertionConsumedCount: session.consumedCharacterCount + ), reason: "Overlay hidden because the focused field changed.") + } + func test_wrapperChurnInsideSessionStillAcceptsSuggestion() { let session = CotabbyTestFixtures.activeSession() let target = CotabbyTestFixtures.focusedInputContext(elementIdentifier: "refreshed-wrapper")