Defer keystroke-time focus refresh off the input event tap#320
Closed
FuJacob wants to merge 1 commit into
Closed
Conversation
The CGEvent tap runs handleInputEvent synchronously before macOS delivers the keystroke, and every text mutation called focusModel.refreshNow() inline. That full AX snapshot resolve scales with the field's text and AX subtree (per-candidate value reads, candidate-discovery and deep-geometry walks), so typing into large or Chromium-based fields added felt latency to each character, worst when editing in the middle of existing text. Defer the refresh to the next main-actor turn via scheduleNonBlockingFocusRefresh(), coalesced so fast typing can't queue a backlog of resolves. The keystroke is never blocked; the snapshot still refreshes before the user's next event (e.g. Tab acceptance) and before the post-debounce generateFromCurrentFocus, which re-reads it anyway. The 80ms poll remains the freshness backstop.
Comment on lines
+173
to
+179
| DispatchQueue.main.async { [weak self] in | ||
| guard let self else { | ||
| return | ||
| } | ||
| self.isFocusRefreshScheduled = false | ||
| self.focusModel.refreshNow() | ||
| } |
Contributor
There was a problem hiding this comment.
The flag is reset to
false before refreshNow() is called, which is the right order for allowing re-coalescing. However, in Swift 6 strict-concurrency mode DispatchQueue.main.async closures are not automatically @MainActor-isolated, so the compiler may warn about accessing self.isFocusRefreshScheduled and self.focusModel from a non-isolated context. The existing schedulePostInsertionRefresh uses the same pattern, so this is consistent with the codebase, but annotating the closure or switching to Task { @MainActor [weak self] in … } would silence any strict-concurrency diagnostics if the project ever enables them.
Suggested change
| DispatchQueue.main.async { [weak self] in | |
| guard let self else { | |
| return | |
| } | |
| self.isFocusRefreshScheduled = false | |
| self.focusModel.refreshNow() | |
| } | |
| Task { @MainActor [weak self] in | |
| guard let self else { | |
| return | |
| } | |
| self.isFocusRefreshScheduled = false | |
| self.focusModel.refreshNow() | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Typing lagged in large or Chromium-based text fields because Cotabby ran a full Accessibility snapshot resolve synchronously inside the CGEvent tap, before macOS delivered each keystroke. The
InputMonitortap callshandleInputEventsynchronously and every text mutation calledfocusModel.refreshNow()inline, so the resolve latency was added to each character. That cost grows with the field's text and AX subtree (per-candidate value reads, candidate-discovery and deep-geometry walks), which is why it showed up most when editing inside existing text. This defers the per-keystroke refresh off the tap so it never blocks keystroke delivery, while keeping the snapshot fresh for acceptance and generation.Validation
This is a timing/threading change with no pure-logic delta, so existing state-transition tests are unaffected and no test asserts the previous synchronous-refresh behavior. Runtime "feel" still wants a manual check by typing into a heavy field (e.g. a long Gmail draft) to confirm the lag is gone.
Linked issues
None.
Risk / rollout notes
scheduleNonBlockingFocusRefresh) instead of synchronously inside the tap. I deferred it rather than deleting it specifically so the snapshot stays fresh for Tab-acceptance reconciliation, sinceacceptSuggestionreadsfocusModel.snapshotwithout its own refresh. Freshness is backstopped by the existing 80ms poll and the post-debounce refresh ingenerateFromCurrentFocus, which re-reads the snapshot anyway.isFocusRefreshScheduledflag so continuous typing collapses to at most one pending resolve rather than a backlog.editableDescendantCandidates, avoid the double-resolve when the field signature changes per keystroke) is possible if this does not fully clear the lag in the heaviest fields.Greptile Summary
This PR fixes keystroke input lag in large or Chromium-based text fields by deferring the per-keystroke Accessibility snapshot resolve off the synchronous CGEvent tap path. Previously,
focusModel.refreshNow()ran inline inside the tap callback before macOS delivered each character, adding full AX-resolve latency to every keystroke.scheduleNonBlockingFocusRefresh(), which dispatches the AX resolve to the next main-run-loop turn viaDispatchQueue.main.async, replacing three directrefreshNow()call sites inSuggestionCoordinator+Input.swift.isFocusRefreshScheduledcoalescing flag onSuggestionCoordinatorso continuous rapid typing collapses to at most one pending AX resolve rather than queuing a backlog.Confidence Score: 4/5
Safe to merge; the deferred refresh reliably runs on the next main-loop iteration before any human Tab press, and the 80ms poll backstops freshness for acceptance.
The change is narrowly scoped to timing: the AX resolve still happens on the main actor, all state mutations remain single-threaded, and the coalescing flag is accessed only from @MainActor-isolated code. The two observations — DispatchQueue.main.async vs Task { @mainactor in … } for strict-concurrency hygiene, and the isFocusRefreshScheduled flag not being cleared in stop() — are both edge-case quality notes rather than present defects.
The stop() method in SuggestionCoordinator+Lifecycle.swift is worth a second look: it doesn't reset isFocusRefreshScheduled, so a rapid stop-then-restart cycle could leave the first post-restart keystroke's refresh silently coalesced away until the stale queued block drains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant macOS participant InputMonitor as InputMonitor (tap) participant Coordinator as SuggestionCoordinator participant MainQueue as Main Queue (async) participant FocusModel as FocusModel macOS->>InputMonitor: CGEvent tap fires (synchronous) InputMonitor->>Coordinator: handleInputEvent(event) alt event is acceptance (Tab) Coordinator->>Coordinator: acceptCurrentSuggestion() Coordinator->>FocusModel: reads snapshot (no refresh) else event.shouldSchedulePrediction (text mutation) Note over Coordinator: OLD: focusModel.refreshNow() inline — blocks keystroke delivery Coordinator->>Coordinator: scheduleNonBlockingFocusRefresh() alt "isFocusRefreshScheduled == false" Coordinator->>Coordinator: "isFocusRefreshScheduled = true" Coordinator->>MainQueue: DispatchQueue.main.async else "isFocusRefreshScheduled == true (coalesced)" Coordinator-->>Coordinator: guard return (skip duplicate) end Coordinator->>Coordinator: schedulePrediction() end Coordinator-->>InputMonitor: return InputMonitor-->>macOS: return (keystroke delivered instantly) macOS->>macOS: Delivers keystroke to focused app MainQueue->>Coordinator: async block fires (next run-loop turn) Coordinator->>Coordinator: "isFocusRefreshScheduled = false" Coordinator->>FocusModel: refreshNow() (AX resolve off tap path)Reviews (1): Last reviewed commit: "Defer keystroke-time focus refresh off t..." | Re-trigger Greptile