feat: add Retype Mode for remote-desktop/VDI dictation targets#562
feat: add Retype Mode for remote-desktop/VDI dictation targets#562gabornyergesX wants to merge 1 commit into
Conversation
github-actions
Bot
added
needs PR template
|
The PR Policy check is blocking this PR because required template information is missing. Please update the PR description with:
Visual files detected:
Screenshots or video are required for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes. If this PR has no visual changes, check the no-visual-change box in the template. If this remains incomplete for 48 hours after opening, the PR may be closed. |
gabornyergesX
force-pushed
the
feature/retype-mode
branch
from
78472d9 to
ff82784
Compare
github-actions
Bot
removed
needs PR template
gabornyergesX
force-pushed
the
feature/retype-mode
branch
from
ff82784 to
0dee66c
Compare
|
![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: 0dee66c6e9
ℹ️ 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 rampSucceeded = self.postUnicodeChunks( | ||
| Array(utf16Array[0..<rampEnd]), | ||
| destinationDescription: "PID \(targetPID) (ramp)", | ||
| chunkSize: 1, |
There was a problem hiding this comment.
When Retype Mode handles text containing non-BMP characters such as emoji or some CJK extensions, chunkSize: 1 makes unicodeChunkEnd split the UTF-16 surrogate pair: it backs off from the high surrogate boundary, but then max(end, start + 1) still returns a one-unit chunk. keyboardSetUnicodeString then receives invalid surrogate halves in separate CGEvents, so those characters can be dropped or garbled even though the existing bulk path preserved them. Use scalar-aware chunking or allow the pair to be sent as one chunk.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c86b85a — unicodeChunkEnd now widens forward instead of backing off when the backoff would produce an empty chunk (i.e. at chunkSize: 1), so the surrogate pair is kept together as a single 2-unit chunk instead of splitting into two lone-surrogate CGEvents. Verified with a standalone simulation against "a🎉b": chunks are now ["a"], ["🎉" as one 2-unit chunk], ["b"] at chunkSize 1, and the bulk chunkSize-200 path is unchanged.
| var slowTypeWarmupMs: Double { | ||
| get { | ||
| let value = self.defaults.double(forKey: Keys.slowTypeWarmupMs) | ||
| return value > 0 ? value : 350.0 |
There was a problem hiding this comment.
When a user sets Warm-up Delay to 0 ms (the slider and slowTypeWarmupMsRange explicitly allow 0), this getter reads the stored value as 0 and substitutes the default 350.0, so the zero setting cannot persist and insertTextSlowly still sleeps. The same value > 0 fallback pattern is used for ramp and steady delays; check whether the key exists instead of treating zero as unset.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c86b85a — switched all three Double getters (slowTypeWarmupMs, slowTypeRampDelayMs, slowTypeSteadyDelayMs) to the same presence-check pattern slowTypeRampCharCount already used (defaults.object(forKey:) as? NSNumber), instead of defaults.double(forKey:) > 0. A user-set 0ms now persists correctly instead of being indistinguishable from "unset" and silently falling back to the default.
gabornyergesX
force-pushed
the
feature/retype-mode
branch
from
0dee66c to
c86b85a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c86b85a108
ℹ️ 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 mode == .slowType { | ||
| // insertTextSlowly performs its own warm-up delay; an outer settle | ||
| // delay here would just double it. | ||
| return 0 |
There was a problem hiding this comment.
When Retype Mode is used through a call path that does not pass preferredTargetPID (for example RewriteModeService.acceptRewrite hides Fluid and then calls typeTextInstantly(_:)), returning 0 here removes the no-PID focus-settle delay that standard insertion still uses. The Retype warm-up does not replace that delay because insertTextSlowly captures targetPID before sleeping, so it can post the rewrite to Fluid or another stale focused app instead of the app that is still being restored; keep a pre-capture settle when preferredTargetPID is nil.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a1ea553 — extracted the settle-delay logic into a testable static func settleDelayMs(mode:preferredTargetPID:). It now keeps the 200ms (Retype/Standard) / 80ms (Clipboard Paste) pre-capture settle whenever preferredTargetPID is nil, exactly like the non-Retype path did, and only skips it when the caller already knows the target PID. Added regression tests in RetypeModeFocusSettleTests.swift (6 cases covering all 3 modes × known/unknown PID).
| let rampEnd = Self.unicodeChunkEnd(in: utf16Array, start: 0, chunkSize: settings.slowTypeRampCharCount) | ||
| let rampSucceeded = self.postUnicodeChunks( | ||
| Array(utf16Array[0..<rampEnd]), |
There was a problem hiding this comment.
When the user sets Ramp Length to 0 (allowed by the setting range and UI slider), this still calls unicodeChunkEnd with chunkSize: 0; the helper returns start + 1, so the ramp path sends one UTF-16 unit with the ramp delay instead of none. The fresh failure path is the allowed zero ramp length before postUnicodeChunks: if the text starts with an emoji/surrogate pair, the 0..<rampEnd slice contains only the high surrogate, so the pair-preserving loop never sees both halves and can garble or drop the first character.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a1ea553 — extracted a testable static func rampEnd(for:rampCharCount:). It now short-circuits to 0 when rampCharCount is 0 instead of calling into unicodeChunkEnd (whose max(end, start + 1) forward-progress guard was flooring a zero-length ramp to a 1-unit slice, which could be a lone high surrogate). Added 4 regression tests in RetypeModeChunkingTests.swift, including one that specifically checks a zero ramp length with a leading surrogate pair defers the whole pair to the steady phase.
gabornyergesX
force-pushed
the
feature/retype-mode
branch
from
c86b85a to
d8c3278
Compare
Types dictated text one character at a time with a warm-up pause and a slow-ramp-then-steady per-character delay, for remote-desktop/VDI targets where fast keystroke/unicode injection gets garbled or dropped because the remote keyboard channel needs a moment to sync session/layout state. Reuses the existing chunked-CGEvent posting infrastructure instead of adding a separate insertion path.
gabornyergesX
force-pushed
the
feature/retype-mode
branch
from
d8c3278 to
a1ea553
Compare
1 participant
Description
Adds a new opt-in text insertion mode, "Retype Mode", that types dictated
text one character at a time with a warm-up pause and a slow-ramp-then-steady
per-character delay. This is for remote-desktop/VDI dictation targets where
the default instant-insertion path garbles or drops leading characters
because the remote keyboard channel needs a moment to sync session/layout
state before it reliably forwards fast key events. The existing "Clipboard
Paste" mode doesn't help in these sessions either, since some remote sessions
disable/block the clipboard.
Implementation reuses the existing chunked-
CGEventposting helper(
postUnicodeChunksinTypingService.swift) by parameterizing it with aconfigurable chunk size and inter-chunk delay, instead of duplicating the
event-creation logic in a separate code path. Warm-up delay, ramp length,
ramp pace, and steady pace are all user-tunable in Settings.
Type of Change
Related Issue or Discussion
#563
Testing
swiftlint --strict --config .swiftlint.yml Sourcesswiftformat --config .swiftformat Sourcesxcodebuild ... test— 90/90 tests pass (1 pre-existing, unrelated failure inDictationE2ETestspresent before this PR)Manually verified against a real remote-desktop session — with Retype Mode
enabled, previously-garbled leading characters are inserted correctly.
Debug build compiles cleanly (
xcodebuild -project Fluid.xcodeproj -scheme Fluid -configuration Debug build) andswiftlint --strictnow passes with0 violations across all 128 files (the original patch pushed
SettingsView'sstruct body over the 2000-line limit; the new
slowTypeTuningControls()helper was moved into the existing same-file
private extension SettingsViewblock, following this repo's established pattern for splitting large view
bodies — see
AISettingsView+AIConfiguration.swift).swiftformatwas runscoped to just this PR's 3 touched files: the only change it wanted inside
this PR's own code was demoting a floating (non-declaration-attached) doc
comment to a plain comment in
TypingService.swift, now applied. (Runningit unscoped across all of
Sourcesreformats ~55 unrelated pre-existingfiles — CI doesn't gate on
swiftformatat all, onlyswiftlint --strict,so that whole-repo drift is out of scope here.) No automated tests cover
TextInsertionMode/TypingServiceinsertion paths in this repo.Screenshots / Video
Attach screenshots or a video for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes.
The new "Retype Mode" entry in Settings → Text Insertion Mode, with its
warm-up/ramp/steady sliders.
Demo of Retype Mode inserting text character-by-character into a browser address bar (Chrome, Incognito): the sentence "This is a demo of the retyping feature." is typed in visibly one character at a time and lands correctly, with no dropped or scrambled leading characters.
Notes
Reuses existing
postUnicodeChunkschunking/CGEvent infrastructure ratherthan adding a parallel insertion code path — smaller diff, one path to
maintain.
Update: addressed both automated review findings from Codex
(1,
2):
unicodeChunkEnd's surrogate-pair guard now widens forward instead ofbacking off when backing off would produce an empty chunk (only reachable
at
chunkSize: 1, i.e. Retype Mode) — a surrogate pair (emoji, some CJKextensions) no longer gets split across two CGEvents.
slowTypeWarmupMs/slowTypeRampDelayMs/slowTypeSteadyDelayMsnow usethe same presence-check pattern
slowTypeRampCharCountalready used(
defaults.object(forKey:) as? NSNumber) instead ofdefaults.double(forKey:) > 0, so a user-set0mspersists instead ofbeing indistinguishable from "unset."
Added two regression test files covering both
(
Tests/FluidDictationIntegrationTests/RetypeModeChunkingTests.swift,RetypeModeSettingsTests.swift) — verified they fail against the pre-fixlogic and pass against the fix.
Update 2: addressed two more Codex findings
(3,
4):
TypingService.settleDelayMs(mode:preferredTargetPID:). RetypeMode was unconditionally skipping the pre-capture focus-settle delay,
reasoning its own warm-up covered the same need — but that warm-up runs
after
insertTextSlowlyresolvestargetPID, so a caller that hidesFluid's own window and relies on
preferredTargetPID == nil(e.g.RewriteModeService.acceptRewrite) could capture a stale PID and type intothe wrong app. The settle delay is now preserved whenever the target PID
isn't already known, matching the other modes.
TypingService.rampEnd(for:rampCharCount:). A Ramp Length of0(a valid, slider-reachable setting) still floored to a 1-unit ramp slice
via
unicodeChunkEnd's forward-progress guard — if the dictated textstarted with an emoji/surrogate pair, that 1-unit slice was a lone high
surrogate.
rampEndnow short-circuits to0before calling intounicodeChunkEndat all.Added
RetypeModeFocusSettleTests.swift(6 tests) and 4 more cases toRetypeModeChunkingTests.swiftcovering the ramp-boundary fix — 21regression tests total across the three new test files. Verified both fixes
the same way as Update 1: reverted just the fix logic, confirmed the new
tests fail, restored the fix.