From b9359f82f722f2ae434a8500698ee3e5f89864f7 Mon Sep 17 00:00:00 2001 From: Jacob Fu <141651335+FuJacob@users.noreply.github.com> Date: Wed, 27 May 2026 04:13:40 -0700 Subject: [PATCH] Fix CI: add CaretGeometrySelector type referenced by tests FocusSnapshotResolverSelectionTests referenced a CaretGeometrySelector type that was never committed in #321, breaking the Tests workflow on main. Extract the caret-geometry trust policy out of FocusSnapshotResolver into a pure CaretGeometrySelector in Support/, exposing shouldSearchDeep and select. Wire the resolver through it. Per the tests, primary .derived geometry is now trusted over a deep .exact result and skips the deep AX walk entirely, matching shouldSearchDeep. --- Cotabby.xcodeproj/project.pbxproj | 4 + .../Focus/FocusSnapshotResolver.swift | 94 ++++--------------- Cotabby/Support/CaretGeometrySelector.swift | 89 ++++++++++++++++++ 3 files changed, 112 insertions(+), 75 deletions(-) create mode 100644 Cotabby/Support/CaretGeometrySelector.swift diff --git a/Cotabby.xcodeproj/project.pbxproj b/Cotabby.xcodeproj/project.pbxproj index 21dd475..1ac1d63 100644 --- a/Cotabby.xcodeproj/project.pbxproj +++ b/Cotabby.xcodeproj/project.pbxproj @@ -82,6 +82,7 @@ 6E01052209B73D7361C12CEF /* ClipboardContentDistiller.swift in Sources */ = {isa = PBXBuildFile; fileRef = 96495E4147D828C0B1B22765 /* ClipboardContentDistiller.swift */; }; 6E49ADEB31D04DC77A47DEB0 /* FileLogHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE8C2569A8217EE9BD3B197F /* FileLogHandler.swift */; }; 744B06C2488156B178675615 /* PermissionManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85BF316556FDA64CB8AD07B6 /* PermissionManager.swift */; }; + 76FD91607794883F8E121450 /* CaretGeometrySelector.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */; }; 7B6A63F5DCC2C163CDFD2A5C /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = BC4F887528AE74AC0DD30314 /* Assets.xcassets */; }; 7D6BB9AF72F7076A4E5EE96F /* DownloadableModelCatalogView.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5C2AE9A7E55495D26AD074 /* DownloadableModelCatalogView.swift */; }; 7E9413CE7C999C4612348248 /* SuggestionSessionReconcilerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9C8F07AC52C7A482F5FE34C5 /* SuggestionSessionReconcilerTests.swift */; }; @@ -305,6 +306,7 @@ DEB16474A67CE1D210B944C9 /* SuggestionSubsystemContracts.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SuggestionSubsystemContracts.swift; sourceTree = ""; }; E1D2782B6C7BE3F56BCB22DE /* LlamaVisualContextSummarizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LlamaVisualContextSummarizer.swift; sourceTree = ""; }; E217A66717D78E1E49350EC8 /* DownloadOutcomeClassifierTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DownloadOutcomeClassifierTests.swift; sourceTree = ""; }; + E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CaretGeometrySelector.swift; sourceTree = ""; }; E43E587E421AF544A8300CE4 /* CustomRulesCatalog.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomRulesCatalog.swift; sourceTree = ""; }; E6423D6CC8CC371D2DA899DE /* PermissionOverlayTracker.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionOverlayTracker.swift; sourceTree = ""; }; E7F42112F14026E6253BB865 /* PermissionAndContextModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PermissionAndContextModelTests.swift; sourceTree = ""; }; @@ -591,6 +593,7 @@ 352AF5B2834FEE1F597394E4 /* ApplicationBundleMetadata.swift */, AC70775535A3428991025AB8 /* AXHelper.swift */, AA33F5FFAC5B99384E15CE3E /* BundledRuntimeLocator.swift */, + E3C84377F352140759B448C9 /* CaretGeometrySelector.swift */, 96495E4147D828C0B1B22765 /* ClipboardContentDistiller.swift */, D3A2AC525DC664DB540D4F19 /* ClipboardRelevanceFilter.swift */, C7B2D34A6F3AC9DFD61350F7 /* CotabbyDebugOptions.swift */, @@ -744,6 +747,7 @@ C4C6734678797669055988E0 /* AppUpdateManager.swift in Sources */, 66C23A7C2FCDE0266FF425F8 /* ApplicationBundleMetadata.swift in Sources */, 3CBBC3BFAC0DC8952EE24EF7 /* BundledRuntimeLocator.swift in Sources */, + 76FD91607794883F8E121450 /* CaretGeometrySelector.swift in Sources */, 6E01052209B73D7361C12CEF /* ClipboardContentDistiller.swift in Sources */, D2CAA59F69BCBC07DDA2B0D8 /* ClipboardContextProvider.swift in Sources */, 157A55EB796BEB7819B90D5D /* ClipboardRelevanceFilter.swift in Sources */, diff --git a/Cotabby/Services/Focus/FocusSnapshotResolver.swift b/Cotabby/Services/Focus/FocusSnapshotResolver.swift index 663f426..96f1899 100644 --- a/Cotabby/Services/Focus/FocusSnapshotResolver.swift +++ b/Cotabby/Services/Focus/FocusSnapshotResolver.swift @@ -119,26 +119,25 @@ struct FocusSnapshotResolver { } // The input target and the geometry source don't need to be the same element. - // Native AppKit apps give exact caret rects on the input target itself. Chrome's - // AXTextArea, by contrast, answers BoundsForRange(loc-1, 1) with a multi-line union - // rect — labeled `.derived` here but actually unusable. The leaf AXStaticText holding - // the active line carries its own zero-length selection range and - // AXSelectedTextMarkerRange, so the deep BFS in `resolveDeepGeometrySource` can - // synthesize a real `.exact` rect via Branch 1.5 (TextMarker) on that leaf. - // - // Precedence: - // 1. primary `.exact` (single API call, perfect — no walk needed) - // 2. deep `.exact` (beats primary `.derived` so we escape Chrome's union-rect trap) - // 3. primary `.derived` - // 4. deep `.derived` - // 5. primary `.estimated` / unknown fallback - // The walk is skipped entirely when primary is already `.exact`, and otherwise throttled to - // one BFS per `deepWalkThrottleInterval` while focus stays in the same field. Chromium - // editors keep the focused element at `.derived`, so without the throttle the ~200-node walk - // ran on every keystroke and pinned a CPU core. Within the window we reuse the previous deep - // result, which can trail the live caret by up to one throttle interval of fast typing. + // Native AppKit apps give exact caret rects on the input target itself. The deep BFS in + // `resolveDeepGeometrySource` can recover a real `.exact` rect from a leaf AXStaticText + // (via Branch 1.5 (TextMarker) on its zero-length selection range) when the focused input + // only exposes weak geometry. Selection precedence and the search decision live in the + // pure `CaretGeometrySelector`: + // 1. primary `.exact` (single API call, perfect — no walk needed) + // 2. primary `.derived` (trusted; the walk is skipped entirely for it) + // 3. deep (any) (only reached when primary is `.estimated`/unknown) + // 4. primary (any, fallback) + // The walk is skipped whenever primary geometry is already trustworthy (`.exact`/`.derived`), + // and otherwise throttled to one BFS per `deepWalkThrottleInterval` while focus stays in the + // same field, so the ~200-node walk does not run on every keystroke and pin a CPU core. + // Within the window we reuse the previous deep result, which can trail the live caret by up + // to one throttle interval of fast typing. let deepResult: CaretGeometryResult? - if resolvedCandidate.caretQuality == .exact { + if !CaretGeometrySelector.shouldSearchDeep( + primaryRect: resolvedCandidate.caretRect, + primaryQuality: resolvedCandidate.caretQuality + ) { deepResult = nil } else { deepResult = deepWalkThrottle.result( @@ -153,7 +152,7 @@ struct FocusSnapshotResolver { } } - guard let caret = Self.selectCaretGeometry( + guard let caret = CaretGeometrySelector.select( primaryRect: resolvedCandidate.caretRect, primaryQuality: resolvedCandidate.caretQuality, primaryObservedCharWidth: resolvedCandidate.observedCharWidth, @@ -270,61 +269,6 @@ struct FocusSnapshotResolver { return ordered } - /// Chooses the caret geometry to ship from the primary candidate and the optional deep-tree - /// result, following a fixed precedence (see the call site). Pulled out of `resolveSnapshot` - /// so that method stays under the cyclomatic-complexity limit; returns `nil` when neither - /// source produced a rect, which the caller maps to an unsupported snapshot. - /// - /// Precedence: primary `.exact` → deep `.exact` (beats primary `.derived`, escaping Chrome's - /// union-rect trap) → primary `.derived` → deep (any) → primary (any, fallback). - private struct SelectedCaretGeometry { - let rect: CGRect - let source: String - let quality: CaretGeometryQuality - let observedCharWidth: CGFloat? - } - - private static func selectCaretGeometry( - primaryRect: CGRect?, - primaryQuality: CaretGeometryQuality?, - primaryObservedCharWidth: CGFloat?, - deepResult: CaretGeometryResult? - ) -> SelectedCaretGeometry? { - if let primary = primaryRect, primaryQuality == .exact { - return SelectedCaretGeometry( - rect: primary, source: "exact primary", quality: .exact, - observedCharWidth: primaryObservedCharWidth - ) - } - if let deep = deepResult, deep.quality == .exact { - return SelectedCaretGeometry( - rect: deep.rect, source: "exact deep", quality: .exact, - observedCharWidth: deep.observedCharWidth - ) - } - if let primary = primaryRect, primaryQuality == .derived { - return SelectedCaretGeometry( - rect: primary, source: "derived primary", quality: .derived, - observedCharWidth: primaryObservedCharWidth - ) - } - if let deep = deepResult { - return SelectedCaretGeometry( - rect: deep.rect, source: "\(deep.quality.label) deep", quality: deep.quality, - observedCharWidth: deep.observedCharWidth - ) - } - if let primary = primaryRect { - return SelectedCaretGeometry( - rect: primary, - source: "\(primaryQuality?.label ?? "unknown") primary-fallback", - quality: primaryQuality ?? .estimated, - observedCharWidth: primaryObservedCharWidth - ) - } - return nil - } - /// Runs deep geometry search from the resolved editable candidate first, then falls back to /// the raw focused node when those are different branches of the same local AX neighborhood. private func resolveDeepGeometrySource( diff --git a/Cotabby/Support/CaretGeometrySelector.swift b/Cotabby/Support/CaretGeometrySelector.swift new file mode 100644 index 0000000..683d22f --- /dev/null +++ b/Cotabby/Support/CaretGeometrySelector.swift @@ -0,0 +1,89 @@ +import CoreGraphics +import Foundation + +/// Pure caret-geometry trust policy used by `FocusSnapshotResolver`. +/// +/// Two decisions live here, kept free of live Accessibility objects so they can be unit tested: +/// 1. `shouldSearchDeep` — whether the focused input's own caret geometry is too weak to trust, +/// so the resolver should run the expensive deep AX-tree walk for a better source. +/// 2. `select` — given the primary candidate's geometry and an optional deep-walk result, which +/// rect to actually ship, following a fixed precedence. +/// +/// The regression these protect against is trusting a descendant rect over the focused input's own +/// usable rect (or vice versa). Chrome's AXTextArea answers BoundsForRange with a multi-line union +/// rect — labelled `.derived` but unusable for precise caret placement — while the leaf AXStaticText +/// holding the active line carries a real `.exact` rect that the deep walk can recover. +enum CaretGeometrySelector { + /// The caret geometry chosen to ship, plus a human-readable source label for diagnostics. + struct Selected: Equatable { + let rect: CGRect + let source: String + let quality: CaretGeometryQuality + let observedCharWidth: CGFloat? + } + + /// Whether the primary (focused-input) caret geometry is too weak to trust, so the resolver + /// should run the deep AX-tree walk for a more precise source. + /// + /// `.exact` and `.derived` are trusted as-is; only `.estimated`, unknown quality, or a missing + /// rect justify the ~200-node walk. The walk pins a CPU core when run on every keystroke, so we + /// avoid it whenever the primary geometry is already good enough. + static func shouldSearchDeep( + primaryRect: CGRect?, + primaryQuality: CaretGeometryQuality? + ) -> Bool { + guard primaryRect != nil else { + return true + } + switch primaryQuality { + case .exact, .derived: + return false + default: + return true + } + } + + /// Chooses the caret geometry to ship from the primary candidate and the optional deep-tree + /// result. Returns `nil` when neither source produced a rect, which the caller maps to an + /// unsupported snapshot. + /// + /// Precedence: + /// 1. primary `.exact` (single API call, perfect — no walk needed) + /// 2. primary `.derived` (trusted; `shouldSearchDeep` skips the walk entirely for it) + /// 3. deep (any) (only reached when primary is `.estimated`/unknown) + /// 4. primary (any, fallback) + static func select( + primaryRect: CGRect?, + primaryQuality: CaretGeometryQuality?, + primaryObservedCharWidth: CGFloat?, + deepResult: CaretGeometryResult? + ) -> Selected? { + if let primary = primaryRect, primaryQuality == .exact { + return Selected( + rect: primary, source: "exact primary", quality: .exact, + observedCharWidth: primaryObservedCharWidth + ) + } + if let primary = primaryRect, primaryQuality == .derived { + return Selected( + rect: primary, source: "derived primary", quality: .derived, + observedCharWidth: primaryObservedCharWidth + ) + } + if let deep = deepResult { + return Selected( + rect: deep.rect, source: "\(deep.quality.label) deep", quality: deep.quality, + observedCharWidth: deep.observedCharWidth + ) + } + if let primary = primaryRect { + return Selected( + rect: primary, + source: "\(primaryQuality?.label ?? "unknown") primary-fallback", + quality: primaryQuality ?? .estimated, + observedCharWidth: primaryObservedCharWidth + ) + } + return nil + } +}