Make clipboard paste the default - #811
Conversation
Greptile SummaryThe PR makes clipboard-based paste the default insertion method and adds coordinated focus restoration, delivery-failure feedback, clipboard preservation, latency analytics, and integration tests.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 719f3de47b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@greptileai review again based on my replies |
FluidVoice PR build readyDownload FluidVoice-PR-811-2efaa124fdc4 The artifact contains the ad-hoc-signed app ZIP, Xcode archive, build manifest, and installation instructions. It expires 5 days after the build. Install the app
This build has its own app identity, so its permissions are separate from the release version of FluidVoice. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6542d82fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| @MainActor | ||
| final class SystemPasteCommandPoster: PasteCommandPosting { | ||
| private static let vKeyCode: CGKeyCode = 0x09 |
There was a problem hiding this comment.
On keyboard layouts where virtual key code 0x09 does not produce “v”, this posts Command plus a different character instead of Command+V. The previous implementation resolved the key code through the current TIS layout, but this hard-coded replacement still returns success after posting, so the coordinator reports the transcript as delivered even though no paste occurred. Restore layout-aware lookup, including reevaluation after runtime layout switches.
Useful? React with 👍 / 👎.
| @MainActor | ||
| final class PasteDeliveryCoordinator { | ||
| static let shared = PasteDeliveryCoordinator() | ||
| nonisolated static let defaultSettlementDelayNanoseconds: UInt64 = 500_000_000 |
There was a problem hiding this comment.
When a busy or asynchronous target handles the posted Command+V more than 500 ms later, settlement has already restored the original clipboard, so the target pastes the old contents instead of the transcript while delivery was reported successful. The newly added testDefaultSettlementDelayIsLongEnoughForSlowPasteTargets at Tests/FluidDictationIntegrationTests/PasteDeliveryCoordinatorTests.swift:228-230 explicitly requires 1.5 seconds, so the current constant also makes that test fail.
Useful? React with 👍 / 👎.
c6542d8 to
6169400
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6169400fd6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad2a01b3a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5dd3ea1ff0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let isStillFocused = context.pid == TypingService.currentFocusedPID() && | ||
| (context.element == nil || TypingService.isCapturedFocusStillActive(context)) | ||
| return (context.pid, !isStillFocused) |
There was a problem hiding this comment.
When focus moves to another editable element in the same application during recording, this can incorrectly decide that the original field is still focused: isCapturedFocusStillActive ultimately accepts any text-like AX element owned by the same PID, even when it is not CFEqual to the captured element. The restore is then skipped and the global Cmd+V lands in the newly focused field rather than the recorded destination; require exact element identity here or always prepare the captured target when the focused element changed.
Useful? React with 👍 / 👎.
| private static func restoreExactTarget(_ context: RecordingTargetContext) async -> Bool { | ||
| guard AXIsProcessTrusted(), context.window != nil, context.element != nil else { return false } |
There was a problem hiding this comment.
For applications that expose the focused AX element but not kAXFocusedWindowAttribute or kAXMainWindowAttribute, the captured context legitimately has window == nil; this guard rejects it before attempting to set kAXFocusedAttribute on the usable element, and the recovery guard rejects it again. After any focus displacement, delivery therefore reports targetRestoreFailed even though the exact field could still be restored; require the element and treat the window raise as optional, as the previous restoration path did.
Useful? React with 👍 / 👎.
|
This pull request has been marked stale because it has had no activity for 5 days. It will be closed in 2 days if there is no further activity. Add the |
5dd3ea1 to
f3c8401
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3c84014ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if case let .recoverableFailure(failure) = result { | ||
| self.showTextDeliveryFailure(failure, transcript: text) |
There was a problem hiding this comment.
When Paste Last is invoked through its global shortcut while the recording overlay is hidden, this failure path only stores the error and calls finishProcessingKeepingOverlayVisible(). That method does not present an idle overlay—NotchOverlayManager.setProcessing(false) only updates state—so the retry/copy controls remain invisible and the failed paste appears to do nothing. Explicitly show the overlay before reporting the recoverable failure, or provide feedback through a surface that is already visible.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77261b6ef9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let didWrite = self.pasteboard.writeIntentionalText(lease.text) | ||
| self.log("intentional_copy_settled generation=\(generation) success=\(didWrite)") |
There was a problem hiding this comment.
When “Copy transcription to clipboard” is enabled and the settlement write fails, writeIntentionalText may already have cleared the general pasteboard before returning false, but this result is only logged after self.lease discarded the original snapshot. A transient pasteboard/server write failure can therefore leave the clipboard empty—losing both the user's previous contents and the requested transcript—even though insertion was reported successful; retain the lease and restore or retry when ownership has not moved to an external copy.
Useful? React with 👍 / 👎.
77261b6 to
4cb6683
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cb6683f37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try? await Task.sleep(nanoseconds: 50_000_000) | ||
| guard Self.isExactFocusTargetActive(requiredFocusTarget), | ||
| self.postReturnKey(postInsertionKey, targetPID: preferredTargetPID) | ||
| await self.postReturnKey(postInsertionKey, targetPID: preferredTargetPID) |
There was a problem hiding this comment.
When Spoken Send is used with the new default clipboard path, typeOutputPlanInstantly returns as soon as Cmd+V is posted, so this fixed 50 ms delay can expire before an asynchronous target has inserted the clipboard payload. The Return event can then send the pre-existing draft, with the dictated text appearing afterward in a new draft; confirm that the paste was consumed before dispatching the post-insertion key.
Useful? React with 👍 / 👎.
| if shouldCopyToClipboard, !shouldTypeExternally { | ||
| ClipboardService.copyToClipboard(finalText) |
There was a problem hiding this comment.
When “Copy to Clipboard” is enabled for an external dictation, shouldCopyToClipboard implies shouldTypeExternally, so this branch never performs the advertised automatic backup. The transcript is only made permanent later by a successful delivery settlement; if focus restoration, Accessibility, snapshot capture, or the temporary write fails, the old implementation's clipboard backup is lost and the user must manually press the recovery copy button.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2efaa124fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vDown.post(tap: .cghidEventTap) | ||
| vUp.post(tap: .cghidEventTap) |
There was a problem hiding this comment.
When focus changes after ContentView prepares the recorded field but before this event is posted—for example, while the clipboard snapshot is being materialized—these HID-tap events go to whichever application is currently focused. The coordinator neither receives the preferred target PID nor revalidates the captured element immediately before dispatch, so it can paste the transcript into an unintended application and still return .commandPosted; post to the prepared target and verify its exact focus at dispatch time.
Useful? React with 👍 / 👎.
| deliveryResult = deliveryOutcome.didInsert || deliveryOutcome.didDispatchAction | ||
| ? .commandPosted | ||
| : .recoverableFailure(.pasteCommandFailed) |
There was a problem hiding this comment.
When Spoken Send insertion fails before dispatch because Accessibility is unavailable, clipboard capture/write fails, or target preparation fails, this conversion collapses every failure into .pasteCommandFailed. The recovery UI consequently shows “Could not send the paste command” instead of actionable guidance such as enabling Accessibility or handling an unpreservable clipboard; carry the underlying TextDeliveryFailure through DeliveryOutcome rather than fabricating one here.
Useful? React with 👍 / 👎.
|
This pull request has been marked stale because it has had no activity for 5 days. It will be closed in 2 days if there is no further activity. Add the |
1 participant
Description
Type of Change
Related Issue or Discussion
#802
Testing
swiftlint --strict --config .swiftlint.yml SourcesScreenshots / Video
Attach screenshots or a video for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes.
Notes
Add reviewer context, rollout notes, or known tradeoffs here.