Accept Voice Memos drags and add batch file transcription - #716
Accept Voice Memos drags and add batch file transcription#716shreeraman96 wants to merge 17 commits into
Conversation
github-actions
Bot
added
the
needs screenshots
|
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. |
github-actions
Bot
removed
the
needs screenshots
9f6032a to
d6c509d
Compare
Greptile SummaryThis PR adds promise-aware Voice Memos drops and persistent sequential batch transcription.
|
![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: d6c509d639
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private func stopAndProcessTranscription(route: DictationOutputRoute = .normal) async { | ||
| // Recording session is ending: release dictation intent so the batch arbiter | ||
| // (and any dictation restart) no longer sees dictation as active. | ||
| FileTranscriptionSession.shared.endDictationIntent() |
There was a problem hiding this comment.
For recordings that set dictationIntent, clearing it at the start of the stop path opens a concurrency window: ASRService.stop() sets asr.isRunning to false before it runs the final transcription pass, so a file drop during the visible “Transcribing” phase can enter MeetingTranscriptionService.transcribeFile while the dictation final pass is still using the shared provider. Move this release until after await asr.stop(...) returns (or otherwise keep the batch arbiter locked through the final ASR transcription).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in ebe47e7. stopAndProcessTranscription now releases via defer at function scope instead of clearing at entry, so the flag survives the final transcription pass (and the early returns for empty text/cancellation/failure).
FluidVoice PR build readyDownload FluidVoice-PR-716-c44affd667f8 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: bf2e5424a7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Want your agent to iterate on Greptile's feedback? Start a greploop in Codex and it will work through the open comments and keep going until this PR reviews clean. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e3f4992e2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: 61eb88887e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // Relocation empties the legacy dir, so a sweep would see it as undelivered | ||
| // and delete a destination the OS may still be writing into. | ||
| pendingFallbackDirs: state.hasInFlightWork ? [legacyDir].compactMap(\.self) : [], | ||
| totalExpected: max(receiverDirs.count, state.payloadCountValue(), 1), |
There was a problem hiding this comment.
When a single NSFilePromiseReceiver advertises multiple fileNames and the modern path fails while the legacy/data fallback writes those files, totalExpected is capped at receiverDirs.count (1). That value is passed to selectDelivery(... expectedItemCount:), so only the first fallback file is delivered and the rest of the promised files are silently dropped; track the promised filename count instead of the number of staging dirs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 33bd0f6. This was a bug I introduced with the expectedItemCount bound two commits earlier, and it is the exact mirror of the phantom-extra-file case that bound was added to fix.
ResolutionState now accumulates each receiver's fileNames.count as it registers, and totalExpected uses that instead of receiverDirs.count:
totalExpected: max(state.promisedFileCountValue(), state.payloadCountValue(), 1)Receiver dirs are still counted separately for allReceiversResolved and the pending-dir logic, which are genuinely per-dir concerns.
Regression test asserts a single receiver promising three names delivers all three from the fallback, and pins the failure the receiver-count bound produced (capped at 1, two files silently dropped).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8fcc29c2f1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: 5618d0cfba
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: a907cd5683
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@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: 370aa57949
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- AppKit promise-aware drop target (SwiftUI onDrop cannot receive file promises): concrete URLs + file promises, resolved off-main via modern receiver, legacy promise API, and raw pasteboard data fallbacks - BatchTranscriptionCoordinator: sequential per-item state machine with cancellation, staging cleanup, and batch/dictation arbitration - App-level FileTranscriptionSession so in-flight work survives navigation - Multi-select file picker; batch progress UI with per-item status
NSPasteboard is not thread-safe: our worker's per-item reads raced the promise receivers' internal pasteboard access and crashed in the type cache (NSInternalInconsistencyException -> ggml terminate handler -> abort). All app-side reads now happen first on one thread, receivers start only afterwards on a serial queue, and every pasteboard call is wrapped in an ObjC exception catcher so a future race logs instead of killing the app.
- Single MainActor arbiter for the shared ASR model: dictation intent flag set synchronously before any await closes the TOCTOU gap; batch items wait on dictation intent, live capture, and in-flight single-file transcription - Batch guard moved to every hotkey entry point (dictate/prompt/command/ rewrite), with an audible cue when dictation is blocked mid-batch - Coordinator: pending-driven processing (completed items never re-run when enqueueing after an undismissed batch) and self-draining restart (items enqueued during the cancel window are picked up, not stranded) - CancellationError rethrown unwrapped from transcribeFile (no false failure analytics) - Drop resolver: empty-delivery path no longer deletes pending receiver dirs; receivers hold in-flight tokens so slow (iCloud) promises aren't abandoned at the 30s soft timeout; failed receivers count as resolved so partial failures deliver promptly; partial-failure accounting includes raw-data item counts - Single-file drops route into the batch when anything is in flight or a finished batch is still displayed; drop errors auto-dismiss - 7 new regression tests (182 total)
Review findings on the single-model arbiter: a batch could start concurrent inference, or wait forever on a dictation that had already ended. - Claim intent at the command/rewrite callbacks and in beginDictationRecording, which start ASR directly and previously left both arbitration flags false until isRunning flipped. - Release it after stopAndProcessTranscription completes rather than at entry; ASRService.stop() clears isRunning before the final transcription pass, so an early release let a drop run during the visible "Transcribing" phase. - Release it in ASRService.stopWithoutTranscription, which every cancel path (Escape, overlay cancel, settings window teardown) funnels through, above the isRunning guard so a cancel while starting counts too. - Claim it after the already-running guard in startRecording, so an ignored start cannot latch a flag it never releases.
- Reduce a provider-supplied promise name to one path component; separators or
".." let the raw-data fallback write outside the staging directory.
- Bound selectDelivery by the pasteboard item count: the raw-data path renames
same-named items ("New Recording 2.m4a"), which matched no modern name and was
delivered as a phantom extra file for a two-memo drag.
- Bind items.last instead of force-unwrapping it (swiftlint --strict).
The coordinator deletes an item's staging dir as soon as that item finishes, so it assumes one item owns one dir. Delivery broke that assumption: the raw-data fallback writes every item into one shared dir, the legacy API writes every file into one destination dir, and a receiver carrying several file names fills one dir — and stagingDir was derived from the file's parent path, so N files claimed the same dir. The first item to finish destroyed files the queued items still needed; cancel() hit the same path while an item was transcribing. - Relocate each delivered file into a fresh item dir at delivery, reporting that dir as its owner. A failed move reports no owner rather than a shared dir. - Treat the legacy dir as pending while its writer is in flight: relocation empties it, so the sweep would otherwise delete a destination the OS is still writing into. - Release a receiver's in-flight token once, after its last file callback. receivePromisedFiles fires once per name, so the token was over-decremented, leaving hasInFlightWork false and disabling both the soft-timeout hold and the guard against delivering a half-written multi-item sweep. - Document staging-dir ownership on Request/Item.
Cancellation was only checked before the transcription await, so a provider that returns normally after a cancel still reached the completion writes: the result landed in FileTranscriptionHistoryStore and fired success analytics while the batch card showed the item as cancelled. - Recheck after the native path's transcribeFile, which has no interruption point at all — the UI's "Cancelling…" state exists precisely because that call runs to completion. - Recheck after the chunked loop, which only checks between chunks, so a cancel during the final chunk (up to 20 minutes of audio) reached the writes too. Both throw into the coordinator's existing CancellationError path.
Branch-added comments were 19% of added lines against a repo norm nearer 8%. Collapses prose blocks to single lines and drops change narration, keeping the live-verified AppKit facts that are not inferable from the code: pasteboard main-thread deadlocks, read ordering vs the NSPasteboard type-cache crash, the legacy promise API's 35-80s block, the pending-dir sweep hazard, Cocoa 3072 on Catalyst sources, and the timeout rationale. 404 comment lines to 265. Also moves a dismissBatch() doc comment that had drifted onto showDropError.
totalExpected counted receiver dirs, but one NSFilePromiseReceiver can promise several files. Bounding selectDelivery by that count meant a single receiver advertising three names whose modern path failed delivered only the first fallback file and silently dropped the rest — the mirror of the phantom-extra -file bug the bound was added to fix. ResolutionState now accumulates each receiver's fileNames count; receiver dirs are still counted separately for the resolved/unresolved checks, which are genuinely per-dir.
Second pass over the drop/batch files: deletes comments that restate the code, compresses prose to single lines, and drops doc comments that only repeat a well-named symbol. Every live-verified fact is kept — pasteboard main-thread deadlocks, read ordering vs the NSPasteboard type-cache crash, the legacy promise API's 35-80s block, the pending-dir sweep hazard, staging-dir ownership, and the dictationIntent TOCTOU window. 265 comment lines to 78 across the six files.
A Voice Memos promise can carry metadata the OS refuses to decode; NSFilePromiseReceiver raises rather than reporting an error, and an uncaught NSException aborts the process. Guard both decode points and degrade to the raw-data fallback. Delivery also waited on a receiver Voice Memos only cancels when the next drag starts, so a lone drop sat until the 120s hard timeout. An unresolved receiver that has written nothing past the grace no longer holds delivery open, and receivers get their own in-flight counter so an uncancelled one cannot gate the fallback writers.
A receiver promising several files can fail on one and still have written the rest, but any failure marked the whole dir failed, and failed dirs are never delivered and are swept at once. Only a dir that produced nothing counts as failed now. The legacy path's returned names were logged and discarded, so a legacy-only multi-file drop left totalExpected at 1 and selectDelivery dropped every file after the first.
A command or edit hotkey press during a file batch flipped activeRecordingMode and re-skinned the overlay before the batch guard returned, leaving the app in that mode with nothing recording and isCommandRecordingProvider reporting it to the hotkey manager. The rewrite path also captured the selection and started the rewrite service. The batch guard now runs first, so its blocked cue still plays while a recording is live. The isRunningOrStarting guard stays below the mutations: those mutations are how a cross-mode hotkey press switches an in-flight recording without restarting ASR.
Comment-only. Trims the branch's own comments toward the density target, keeping the why and dropping restatement. Pre-existing comments in shared files are untouched.
Giving receivers their own in-flight counter left hasInFlightWork blind to them, so the 30s soft timeout could fire while a promise receiver was mid-transfer; the timeout path delivers only completed dirs, so that file was reported unreadable. Providers have been seen taking 80s. The loop now holds past the soft timeout while an unresolved receiver has written files, reusing the discriminator the stall fix already relies on, so an idle receiver still cannot gate delivery.
370aa57 to
5cb1d04
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cb1d04e74
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A drop with several receivers where one completes and another stalls without writing satisfies neither early exit: modernComplete needs every receiver resolved, and the fallback path needs no modern wins at all. It waited the full 30s soft timeout to deliver files that were stable seconds earlier. The soft timeout is now 8s once any receiver has completed and 30s while none has, reusing the existing break rather than adding a second notion of settled: that break already holds delivery open for a receiver still writing, and cannot sweep a legacy dir early.
1 participant
Description
Dragging a recording from Apple Voice Memos into File Transcription did nothing, and only one file could be transcribed at a time. This PR makes both work:
PromiseAwareDropView): SwiftUI's.onDropcan never see Voice Memos drags — they are file promises with nopublic.file-url(open Apple gap, FB13583826). An AppKitNSViewRepresentableoverlay now accepts both concrete URLs and file promises. Resolution runs entirely off the main thread (verified live: any main-thread pasteboard read can deadlock intermittently and freezes the system-wide drag session) through three racing paths: modernNSFilePromiseReceiver(completion-callback-gated, so a stalled iCloud download is never delivered truncated) → raw pasteboard data (what Voice Memos actually serves, ~100 ms) → deprecated legacy promise API as a lazy last resort (an eager legacy request holds Voice Memos' promise machinery hostage for 35–80 s and blocks its next drag).BatchTranscriptionCoordinator): sequential per-item state machine — per-file status/progress/error, "no speech" detection, cancellation (pending items stop instantly; the in-flight native transcription runs out, shown as "Cancelling…"), and staging-dir cleanup only after each item finishes.FileTranscriptionSession): coordinator + service moved out of the view so in-flight work survives sidebar navigation.beginDictationRecording, command and rewrite at their callbacks, which start ASR directly) and released only once the stop path finishes —ASRService.stop()clearsisRunningbefore the final transcription pass — withstopWithoutTranscriptionreleasing it for every cancel path.providers.first).Task.checkCancellation()points inMeetingTranscriptionService's chunk loop so a cancelled batch stops mid-file on the chunked path.Staging design: each promised file gets its own temp subdirectory (two memos titled "New Recording.m4a" never collide), duplicates from losing delivery paths are swept — except dirs whose promise is still pending, because deleting a promise's destination mid-write leaves the source app's drag session unresolved and Voice Memos then refuses all further drags until restarted (verified live; cleaned up on a deferred task instead).
Type of Change
Related Issue or Discussion
Closes #219
Testing
swiftlint --strict --config .swiftlint.yml Sources Tests— 0 violations, 151 filesswiftformatreflows ~3k unrelated pre-existing lines on main; new files follow the existing stylePromiseDropSupportTests,BatchTranscriptionCoordinatorTests,HotkeyShortcutTests); 35 new unit tests total. Note: a full-target run currently stops early insideDictationE2ETests.testDictationEndToEnd_whisperTiny_transcribesFixture(Whisper/ggml model load) while still reporting TEST SUCCEEDED — pre-existing, unrelated to this PR, but it means a whole-suite number is not trustworthy right now. Was 208/208 pass (35 new unit tests: coordinator state machine, cancellation, staging cleanup + session-root pruning, drop strategy selection, delivery dedup/sweep, format filtering, promise-name sanitization, alternate-vs-item delivery bounds)Live end-to-end verification on this machine (Voice Memos → FluidVoice):
Screenshots / Video
Batch card after a drop (per-item status, header summary, Done):
Notes
fileNamesfills one dir too). Fell out of that: the legacy dir is now held back from the delivery sweep while its writer is in flight, and a receiver's in-flight token is released once after its last file callback instead of once per file — the over-decrement lefthasInFlightWorkfalse and disabled the guard against delivering a half-written sweep...let the raw-data fallback write outside staging);selectDeliveryis bounded by the pasteboard item count, since the fallback renames same-named items ("New Recording 2.m4a") and that name matched no modern name, so it was delivered as a phantom extra file; and the dictation-intent flag is now claimed and released on every start, stop, and cancel path.NSFilePromiseReceiverpath is unreliable for Voice Memos (a Catalyst app): called on the main thread it fails instantly with Cocoa 3072; on a background thread it completes only sometimes. The raw pasteboard data path is what reliably delivers Voice Memos audio; the receiver + legacy paths cover other promise sources (Photos, Mail).performDragOperationreturns is outside AppKit's documented lifetime contract — it is the only arrangement found that neither deadlocks nor loses drops; commented in code with the poll-timeout safety net.CloudRecordings.dbis an undocumented schema.