From 3b7802e50e75ce2029f76d1b687d13b6a931ddbc Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:15:38 -0400 Subject: [PATCH 1/2] Treat an unreadable title, URL or placeholder as the same field, not navigation Since #832 the window title, page URL and field placeholder are re-read on every focus poll under the 50 ms AX timeout and compared with == inside the session identity. A host busy with a burst of synthetic insertions (Chromium during rapid Tab accepts) times one read out; the nil failed the equality as a focused-field change, FocusTracker advanced the focus sequence for it, the session was torn down, and the next Tab reached the page. Add FocusedInputSessionIdentity.continues: process, bundle and focus sequence must match exactly, while a surface fact agrees unless both reads are known and differ. Use it everywhere a live snapshot is compared with a stored one, apply the same rule in FocusedInputPollingSignature.continuesField, and have FocusTracker carry the last known facts through a blank poll so A -> nil -> B is still detected as navigation. --- .../SuggestionCoordinator+Input.swift | 6 +- .../SuggestionCoordinator+Prediction.swift | 7 +- Cotabby/Models/Focus/FocusModels.swift | 28 ++++++++ Cotabby/Services/Focus/FocusTracker.swift | 7 +- .../Suggestion/State/ContextBuffer.swift | 6 +- .../State/SuggestionInteractionState.swift | 4 +- .../Visual/VisualContextCoordinator.swift | 18 ++--- .../Focus/FocusedInputPollingSignature.swift | 53 +++++++++++++- .../Session/SuggestionContinuationPlan.swift | 2 +- .../Session/SuggestionSessionReconciler.swift | 4 +- ...SuggestionConversationIsolationTests.swift | 4 +- ...SuggestionCoordinatorAcceptanceTests.swift | 64 +++++++++++++++++ .../Models/Focus/FocusModelsTests.swift | 41 +++++++++++ .../State/ContextBufferNavigationTests.swift | 13 +++- .../SuggestionInteractionStateTests.swift | 24 ++++++- .../FocusedInputPollingSignatureTests.swift | 72 ++++++++++++++++--- ...SuggestionSessionReconciliationTests.swift | 49 +++++++++++-- 17 files changed, 363 insertions(+), 39 deletions(-) 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..b8e227f8 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,8 +293,8 @@ final class VisualContextCoordinator { /// visual-context session still belongs to that same field. func excerpt(for context: FocusedInputContext) -> String? { expireExcerptIfNeeded() - guard let activeAugmentationSession, - activeSessionIdentity == context.sessionIdentity, + guard let activeAugmentationSession, let activeSessionIdentity, + context.sessionIdentity.continues(activeSessionIdentity), activeAugmentationSession.elementIdentifier == context.elementIdentifier, activeAugmentationSession.focusChangeSequence == context.focusChangeSequence, activeAugmentationSession.status == .ready 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") From 911d72f5832ac137dc304ed61a2cacfe22f237c0 Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:23:18 -0400 Subject: [PATCH 2/2] Keep exact identity for handing a screen excerpt to a prompt The session-keeping checks tolerate a surface fact the poll failed to read. excerpt(for:) feeds screen text into a request, so a chat switch that reuses the composer, frame and URL while its new title is momentarily unreadable must not condition the new chat on the previous chat's excerpt. Withhold the excerpt for that poll instead; the capture survives for the next readable one. --- Cotabby/Services/Visual/VisualContextCoordinator.swift | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Cotabby/Services/Visual/VisualContextCoordinator.swift b/Cotabby/Services/Visual/VisualContextCoordinator.swift index b8e227f8..487282df 100644 --- a/Cotabby/Services/Visual/VisualContextCoordinator.swift +++ b/Cotabby/Services/Visual/VisualContextCoordinator.swift @@ -293,8 +293,14 @@ final class VisualContextCoordinator { /// visual-context session still belongs to that same field. func excerpt(for context: FocusedInputContext) -> String? { expireExcerptIfNeeded() - guard let activeAugmentationSession, let activeSessionIdentity, - context.sessionIdentity.continues(activeSessionIdentity), + // 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, activeAugmentationSession.focusChangeSequence == context.focusChangeSequence, activeAugmentationSession.status == .ready