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. |
Greptile SummaryThis revision addresses the previous review findings while retaining the PR’s premium overlay and experimental Live Typing features.
Confidence Score: 5/5The PR appears safe to merge; the previous findings are resolved and no actionable regression was identified in the follow-up changes. Live Typing now uses the recording-time focus target, restores clipboard state after fallback, and preserves Spoken Send behavior through the action-only delivery path. The preview default, completion dwell, and scale labels have also been corrected, and all previous threads were resolved. Reviews (2): Last reviewed commit: "fix: address review feedback on live typ..." | Re-trigger Greptile |
![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: 4788c2eb4d
ℹ️ 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".
| case .abort(let reason): | ||
| DebugLogger.shared.info( | ||
| "Live typing stopped: \(reason.rawValue)", | ||
| source: "LiveTyping" | ||
| ) | ||
| self.reset() |
There was a problem hiding this comment.
When Live Typing has already written a partial and the user or target app changes any surrounding text, session.partial returns .abort, but this reset discards hasWritten and all ownership information. A subsequent partial can then start a fresh session, and final delivery can fall through to the normal paste path, leaving the earlier partial in place and inserting the transcript again. Keep a terminal session/tombstone through final delivery so an aborted session that wrote text continues to suppress duplicate insertion.
Useful? React with 👍 / 👎.
| private func beginSession() { | ||
| guard let target = LiveTypingAXTarget.capture(preferredPID: nil) else { |
There was a problem hiding this comment.
If focus changes after recording starts but before the first streaming result arrives, this captures whichever element is focused at that later moment rather than the focus target saved for the recording. Live Typing can consequently insert a partial into an unrelated app or field; the final pipeline then restores the original target, detects a focus mismatch, and suppresses final insertion there, leaving stray text in the wrong field. Pass the captured recording PID/element into the live-typing session instead of resolving unrestricted focus on the first partial.
Useful? React with 👍 / 👎.
| if LiveTypingController.shared.consumeFinalDelivery(plainText: plan.plainText) { | ||
| return .inserted |
There was a problem hiding this comment.
When Live Typing owns the final text and Spoken Send is enabled, this early return bypasses TypingService.typeOutputPlanInstantly, which is the only path that validates focus and posts postInsertionKey. Returning .inserted also makes deliverSpokenSend report failure because didDispatchAction is false, so the text remains unsent every time both features are used together. The live-text handoff still needs to run the post-insertion action path and return its actual delivery outcome.
Useful? React with 👍 / 👎.
| let subrole = self.stringAttribute(kAXSubroleAttribute as CFString) | ||
| let isSecure = subrole == (kAXSecureTextFieldSubrole as String) | ||
| || (subrole ?? "").localizedCaseInsensitiveContains("secure") |
There was a problem hiding this comment.
For an accessibility implementation that exposes readable/settable value and selection attributes but does not expose kAXSubroleAttribute, subrole is nil and this marks the field non-secure, allowing partial transcripts to be streamed into it. That contradicts the conservative isSecure() helper above, which treats an unknown subrole as unavailable specifically to avoid writing into an unrecognized secure field. Use that conservative result here so capability probing falls back to final-only delivery when the subrole cannot be verified.
Useful? React with 👍 / 👎.
| set { | ||
| objectWillChange.send() | ||
| self.defaults.set(newValue.rawValue, forKey: Keys.overlayVisualStyle) | ||
| } |
There was a problem hiding this comment.
On the default floating-overlay configuration, the panel is prepared and sized before Settings is used. Switching to or from Companion changes the root view's intrinsic dimensions, but this setter sends no OverlaySizeChanged, show() only repositions the existing panel, and the emitted CompanionSettingsChanged notification has no observer in the repository. The existing NSPanel therefore retains its old frame and clips or mis-centers the new style until an unrelated size update occurs; trigger the controller's size-and-position refresh when the style changes.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Live Typing safety (Codex P1 / Greptile P1): - Bind the session to the field that was focused when recording started (bindRecordingFocus) instead of resolving "whatever is focused now" when the first partial arrives, and re-check that exact element before every write. A partial can no longer land in a field the user moved to. - Keep a tombstone when a session aborts after it already wrote text, so a later partial can not restart a session over it and the final transcript is not inserted a second time. - Treat an absent or empty AX subrole as secure, matching isSecure(), so an unreadable field is never streamed into. Extracted as a pure, tested helper. - Stop clobbering the clipboard on fallback: the transcript is written as a transient, auto-generated item and the previous contents are restored a moment later unless the user changed the clipboard. Spoken Send (Codex P1 / Greptile P1): - When Live Typing already owns the text, still dispatch the configured post-insertion key with an empty plan and report the real outcome, instead of returning .inserted and leaving the send undone. Settings / overlay: - Restore the upstream default for enableStreamingPreview so upgrading never silently disables the live preview. - Post OverlaySizeChanged when the visual style or Companion size changes, so a visible panel re-measures instead of clipping the new style. - Clear the completion flag after the flash dwell, not before it. - Correct the scale slider endpoint labels to the real 25%-300% band. Tests: 560 total, 1 pre-existing AZERTY failure on the baseline.
github-actions
Bot
removed
needs PR template
|
Thanks @greptile-apps and @chatgpt-codex-connector — all 11 findings were legitimate. They are fixed in Live Typing is now bound to the recording target
Aborted sessions keep their tombstone
Spoken Send is dispatched
The clipboard is no longer overwritten
Unreadable subrole is treated as secure
Upgrade no longer disables live preview
Panel re-measures on style change
Completion flash keeps its dwell
Scale labels
Tests:
|
|
You have reached your Codex usage limits. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1870780ef4
ℹ️ 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".
| // still owe a post-insertion action (Spoken Send). The text is already | ||
| // in the field, so only the key is dispatched - and its real outcome is | ||
| // reported, so a spoken send is not reported as failed. | ||
| if LiveTypingController.shared.consumeFinalDelivery(plainText: plan.plainText) { |
There was a problem hiding this comment.
When Live Typing and Spoken Send are enabled together, this action-only branch treats every true result as a successful final insertion. However, consumeFinalDelivery also returns true when ownership was lost, the final replacement failed, or the value became unreadable—cases where it only copies the final transcript to the pasteboard. If focus remains on the original field, such as after an external edit, the key validation can still pass and send the stale partial or user-edited draft; distinguish successful finalization from suppressed delivery before dispatching the action.
Useful? React with 👍 / 👎.
| if !previous.isEmpty { | ||
| let rollback = NSRange(location: range.location, length: (newText as NSString).length) | ||
| _ = target.replace(range: rollback, with: previous) |
There was a problem hiding this comment.
If the first AX replacement mutates the field but immediate readback verification fails—for example because the target normalizes the value or exposes delayed AX updates—previous is empty, so this guard skips rollback even though newText may now be present. The session still has hasWritten == false, causing final delivery to use the normal paste path and duplicate the transcript; rollback must also replace the first write with an empty string, and an unverifiable rollback should leave a tombstone.
Useful? React with 👍 / 👎.
| self.session = LiveTypingSession( | ||
| targetPID: target.pid, | ||
| anchor: selection.location, | ||
| initialValue: value, | ||
| level: level |
There was a problem hiding this comment.
When dictation starts with a nonempty text selection, only selection.location is retained and the session begins with an owned range of length zero. The first partial is therefore inserted before the selected text, and because that original selection becomes part of trailingText, final reconciliation preserves it rather than replacing it as normal paste delivery would. Carry the initial selection length into the session's first owned range or fall back to final-only delivery for nonempty selections.
Useful? React with 👍 / 👎.
| guard let value = target.value() else { | ||
| self.downgrade(&session) | ||
| self.session = session | ||
| return | ||
| } | ||
|
|
||
| switch session.partial(text, currentValue: value) { |
There was a problem hiding this comment.
If the user moves the caret or changes the selection within the same focused field without first modifying its value, contextIsIntact still passes and the next partial rewrites the old owned range; replace then moves the caret back to the owned range's end. This can hijack the user's cursor and place their next keystroke in the wrong location. Read the current selection here and require session.caretIsAtOwnedEnd(...) before each live write.
Useful? React with 👍 / 👎.
| if NotchOverlayManager.shared.isBottomOverlayVisible { | ||
| NotchContentState.shared.markDeliveryCompleted() |
There was a problem hiding this comment.
finishProcessingAndHideOverlay() is not success-only: ContentView also awaits it for an empty transcription at line 2642 and for a failed rewrite at line 3812. With the floating overlay visible, this unconditional mark displays the success check and adds the completion dwell for those failure/empty paths, giving users a false delivery confirmation; pass the actual outcome or mark completion only at confirmed-success call sites.
Useful? React with 👍 / 👎.
|
No new blocking findings in Tip: You can customize Greptile's behavior for this repo with |
1 participant
Description
Adds a set of premium recording-overlay styles, a Companion overlay style, and an
experimental (default-off) live-typing path, and fixes the issues raised in the
first review round.
This is offered as a graft, not as a demand: take what is useful, drop or gate the
rest. It is a large visual PR, so it is reasonable to only take the visual half.
No ASR change. Parakeet/FluidAudio, the audio-capture engine and the mic tap are
untouched. Every new visual only consumes the audio level the app already publishes,
and live typing only consumes the partial transcript the app already computes.
Included:
shared
AuroraComposition, so the Settings thumbnail and the live overlay draw thesame geometry. Wave, Minimal and a circular Pulse whose ring is a
travelling harmonics wave.
OverlayColorThemegains ahighlightrole and thegradients/spectrum ramp the styles share.
OverlayVisualStyle(not a second window, not anOverlaySize): a chromeless SVG-like character rendered in the overlay, with sixelemental variants (fire / water / wind / earth / aurora / gothic), accessories,
a face that follows the lifecycle, listening ripples and glowing eyes while the mic
is live. It inherits the existing window, positioning and lifecycle code, so it
disappears with the overlay like every other style.
previous large look is now its 100 % reference, with its own 50 %–300 % band.
NotchPresentationPolicynow carries the visualizer canvas and iconpolicy; new Ambient Glow (Beta) presentation (no icon, no label, a wider 88×24
canvas); bar count adapts to the canvas width instead of being fixed.
full,committedChunks,finalOnly— an automatic downgrade ladder, an ownership modelof prefix + owned range + trailing text, per-application capability probing, and a
pasteboard fallback. Gated behind
liveTypingExperimental; no application iscertified.
intensity in the Glow Advanced group, a live-typing toggle with a status row, and
a search-index update.
Review round 1 — what was fixed
field captured when recording started (
bindRecordingFocus) instead of resolving"whatever is focused now" on the first partial, and it re-checks that exact element
before every write. A partial can no longer land in a field the user moved to.
restart a session over the same field and the final transcript is not inserted twice.
never streamed into. The rule is now a pure helper with direct test coverage.
posted through an empty plan on the normal safety path and the real outcome is
reported, instead of returning
.insertedand reporting a failed send.transient/auto-generated item and restores the previous contents a moment later
unless the user changed the clipboard. The transcript is in the dictation history
either way.
enableStreamingPreviewrestored to its upstream default, so upgrading neversilently disables the live preview for users who never touched the setting.
(previously it kept the old frame and clipped the new style).
Type of Change
Related Issue or Discussion
Follows the accepted design conversations in
Discussion #949 — "UI needs a rework."
and
Discussion #963 — "Opt-in typed dictation-companion results and captured-target delivery".
#963is the reason for the live-typing shape in this PR: it argues that a recordingclient must validate the captured target rather than current focus, and that a changed
target must fail instead of redirecting output. Those two rules are what review round 1
pushed this PR into implementing. There is no dedicated tracking issue for the visual
work, so if the maintainers would rather fold it into the in-progress UI sweep, this PR
can be closed or reduced to the visual components without any hard feelings.
Related earlier feature requests: #103 (better notch animation), #743 (animated
voice-reactive notch), #865 (minimal overlay option).
Testing
MACOSX_DEPLOYMENT_TARGETstays 15.0swiftlintis not installed in this environment, so it was not runswiftformatis not installed in this environment, so it was not runxcodebuild test -project Fluid.xcodeproj -scheme Fluid -destination 'platform=macOS'(with
CODE_SIGNING_ALLOWED=NO) → 560 tests, 1 failure, and that failure ispre-existing on
main:HotkeyShortcutTests.testKeyboardPayloadIgnoresStrayMouseButtonField(
"Q"vs"A", an AZERTY-layout artefact present in the clean baseline).New coverage added by this PR:
OverlayPremiumSettingsTests.swift(~90 cases) andLiveTypingSessionTests.swift(32 cases), both wired into the test target.What was actually verified visually. The Debug bundle cannot be driven by
Accessibility automation here, so every visual claim comes from offscreen
ImageRendererpasses over the production components (same structs, sameAuroraComposition, sameCompanionGeometry), plus a real Debug build that wasinstalled and launched. The screenshots below are those renders.
What was not verified: signed-release behaviour, real streaming
Parakeet-to-editor delivery, live typing against any third-party app, multi-display
placement, and the completion-flash timing on a real machine. Live Typing ships
default-off for exactly that reason and self-disables on any doubt.
Screenshots / Video
Offscreen renders of the production components. Top to bottom: styles × formats,
Aurora's three layers across levels and themes, the Companion's variants × lifecycle
states, Companion sizes and accessories, the Pulse ring, and the Ambient Glow notch row.
(The image files live on a separate
pr-975-assetsbranch in the fork so they are notpart of this PR's diff.)
Notes
most likely to need upstream redesign; the visual half is independent and can be taken
on its own.
DynamicNotchKit's presentation clipping, so the glow lives inside the content rather
than around the hardware notch.
BottomWaveformViewis kept but is no longer referenced.AVAudioEngine, no new mic tap; installed preferences are only added to, never reset.Commits
4788c2efeat: premium overlays companion and experimental live typing1870780fix: address review feedback on live typing, settings and overlay