Meeting transcriber#63
Conversation
- Remove isMeetingMode from SettingsStore - Revert onboarding flow to not skip accessibility permission - Revert app name to 'Wispr' (from 'Wispr Steno') - Revert bundle identifier to com.stormacq.mac.wispr - Revert development team to original - Meeting transcription is now a separate menu action users can trigger on-demand - Users can switch between dictation and meeting transcription dynamically This addresses maintainer feedback: the app should be polyvalent, allowing users to easily use both dictation and meeting transcription without choosing a persistent mode during onboarding.
- Fix file permissions (100755 → 100644) on all files - Move tasks/todo.md to .kiro/specs/meeting-transcription/design.md - Revert files that should not be in PR: project.pbxproj, OnboardingFlow.swift, .vscode/settings.json, ModelPaths.swift, SettingsStore.swift - Fix MenuBarControllerTests for new meetingStateManager parameter - Add MeetingAudioEngineTests (8 tests): stream behavior, safe no-ops, capture failure handling, double-start guard - Add MeetingStateManagerTests (14 tests): transcript model formatting, state lifecycle, error handling, copy transcript, double-start prevention
Add NSWindowDelegate conformance to MeetingWindowPanel so that windowWillClose syncs isVisible back to false. Without this, the guard in show() would early-return because isVisible was stale.
MeetingAudioEngine: - Replace per-buffer Task spawning with AsyncStream bridge pattern. Tap callbacks yield into continuations; single consumer tasks on the actor read from the streams. Eliminates hundreds of short-lived tasks under heavy audio load and provides natural backpressure. - Replace @unchecked Sendable with plain Sendable on SystemAudioOutputHandler (final class with immutable let property). MeetingStateManager: - Remove 5 redundant await MainActor.run {} wrappers. Since the class is @mainactor, spawned Tasks inherit isolation — direct property access is correct. - Replace 5 unstructured Task properties with single recordingTask using withTaskGroup. Cancelling the parent cascades to all children. Add cancelRecording() for synchronous cancellation at app termination. wisprApp: - Replace fire-and-forget Task { await msm.stopMeeting() } in applicationWillTerminate with synchronous cancelRecording() call.
sebsto
left a comment
There was a problem hiding this comment.
Hey! Regarding the exportTranscript() method that uses NSSavePanel — it crashes because the app has no file-access entitlements, and NSSavePanel also has focus/activation issues in .accessory (menu-bar-only) apps.
The idiomatic SwiftUI replacement is .fileExporter() — it's a declarative view modifier that handles the save dialog automatically, requires no entitlements (user-selected locations are sandbox-safe by design), and avoids the window activation headaches.
Here's how it would work:
- Create a simple
FileDocumenttype:
import SwiftUI
import UniformTypeIdentifiers
struct TranscriptDocument: FileDocument {
static let readableContentTypes: [UTType] = [.plainText]
let text: String
func fileWrapper(configuration: WriteConfiguration) throws -> FileWrapper {
FileWrapper(regularFileWithContents: Data(text.utf8))
}
init(configuration: ReadConfiguration) throws {
let data = configuration.file.regularFileContents ?? Data()
text = String(decoding: data, as: UTF8.self)
}
init(text: String) { self.text = text }
}- Replace the
NSSavePanelcall with a.fileExporter()modifier onMeetingTranscriptView:
@State private var isExporting = false
// On the Export button:
Button("Export") { isExporting = true }
// On the view:
.fileExporter(
isPresented: $isExporting,
document: TranscriptDocument(text: meetingState.transcript.asPlainText()),
contentType: .plainText,
defaultFilename: "meeting-transcript"
) { result in
if case .failure(let error) = result {
Log.stateManager.error("Export failed: \(error.localizedDescription)")
}
}This removes the need for exportTranscript() on MeetingStateManager entirely — the view handles it declaratively. No entitlements, no focus issues, and it works on sandboxed builds too.
| let text = transcript.asPlainText() | ||
| guard !text.isEmpty else { return } | ||
|
|
||
| let panel = NSSavePanel() |
There was a problem hiding this comment.
When clicking on "export" on the UI, The app crashes with this message
"Unable to display save panel: your app has the User Selected File Read entitlement but it needs User Selected File Read/Write to display save panels. Please ensure that your app's target capabilities include the proper entitlements."
| let floatCount = length / MemoryLayout<Float>.size | ||
| guard floatCount > 0 else { return } | ||
|
|
||
| let samples = Array( |
There was a problem hiding this comment.
let samples = Array(
UnsafeBufferPointer(
start: data.withMemoryRebound(to: Float.self, capacity: floatCount) { $0 },
count: floatCount
))
withMemoryRebound returns a pointer that is only valid inside the closure. Returning $0 and using it outside is undefined behavior — the pointer may be invalidated
by the time Array.init reads from it.
This should be:
let samples = data.withMemoryRebound(to: Float.self, capacity: floatCount) { ptr in
Array(UnsafeBufferPointer(start: ptr, count: floatCount))
}
| while !Task.isCancelled { | ||
| try? await Task.sleep(for: .seconds(1)) | ||
| guard !Task.isCancelled else { break } | ||
| self.elapsedTime = self.transcript.formattedDuration ?? "0:00" |
There was a problem hiding this comment.
self.elapsedTime = self.transcript.formattedDuration ?? "0:00"
MeetingTranscript.formattedDuration returns String (non-optional). The ?? is unnecessary and will generate a compiler warning. Should be:
self.elapsedTime = self.transcript.formattedDuration
| @Environment(MeetingStateManager.self) private var meetingState: MeetingStateManager | ||
| @Environment(UIThemeEngine.self) private var theme: UIThemeEngine | ||
|
|
||
| @State private var scrollProxy: ScrollViewProxy? |
There was a problem hiding this comment.
This is declared but never written to. The ScrollViewReader proxy is used inline. Remove the dead state.
| } | ||
|
|
||
| /// Formats the entire transcript as plain text for export. | ||
| func asPlainText() -> String { |
There was a problem hiding this comment.
A new DateFormatter is allocated for every call, and the method iterates all entries. For a long meeting (hundreds of entries), this is wasteful. Consider a static
cached formatter or pass one in:
func asPlainText() -> String {
let formatter = DateFormatter()
formatter.dateFormat = "HH:mm:ss"
return entries.map { ... }.joined(separator: "\n")
}
Same issue in MeetingTranscriptView.formatTime(_:) — called per row in a LazyVStack.
Also probably an occasion to not repeat yourself and have only one formatter utility method
| } | ||
|
|
||
| /// Checks if Screen Recording permission is available by attempting to enumerate shareable content. | ||
| func checkScreenRecordingPermission() async -> Bool { |
There was a problem hiding this comment.
The method exists but nothing in this PR calls it before attempting system audio capture. Either wire it into the meeting start flow (show UI guidance before the SCK prompt) or remove it to avoid dead code.
|
|
||
| /// Opens System Settings to the Screen Recording privacy pane. | ||
| /// Required for meeting mode system audio capture via ScreenCaptureKit. | ||
| func openScreenRecordingSettings() { |
| @@ -2,6 +2,7 @@ import Foundation | |||
| import AVFAudio | |||
| import ApplicationServices | |||
| import AppKit | |||
| import ScreenCaptureKit | |||
There was a problem hiding this comment.
if we don't need ScreenRecording permission, let's remove this and the functions below
|
|
||
| // MARK: - SF Symbols Extensions | ||
|
|
||
| private extension SFSymbols { |
There was a problem hiding this comment.
waveformalready exists in the mainSFSymbolsenum (it'smenuBarProcessing's value and alsoproviderWhisper)clipboardduplicatesSFSymbols.copywhich is already "doc.on.doc"- These should be added to the central
SFSymbols.swiftfile, not in a private extension
|
@gbrunoo I merged #62 which changes the layout of the source files in the project. This will impact this PR, you'll need to move files around and use the new Xcode project layout. |
- Resolve file-location conflicts for meeting feature files (wispr/ → Sources/WisprApp/) - Move MeetingTranscriptView and MeetingWindowPanel to Sources/WisprApp/UI/Meeting/ - Update test imports to @testable import WisprApp + import WisprCore - Resolve content conflict in MenuBarControllerTests.swift
- Replace NSSavePanel with SwiftUI .fileExporter() (fixes crash from missing file-access entitlement and focus issues in .accessory apps) - Add TranscriptDocument FileDocument type for declarative export - Fix undefined behavior: move Array construction inside withMemoryRebound closure (pointer was escaping its valid scope) - Remove unnecessary ?? on non-optional formattedDuration - Remove dead @State scrollProxy property - Cache DateFormatter as static let; add shared MeetingTranscript.formatTime() utility to eliminate per-call allocation in view and model - Remove dead code: checkScreenRecordingPermission(), openScreenRecordingSettings(), and import ScreenCaptureKit from PermissionManager - Deduplicate SF Symbols: remove private extension, use central SFSymbols.swift, add stopFill as the only genuinely new symbol - Add safety comment on nonisolated(unsafe) let inputBuffer explaining why it is safe (synchronous converter callback)
- Replace NSSavePanel with SwiftUI .fileExporter() (fixes crash from missing file-access entitlement and focus issues in .accessory apps) - Add TranscriptDocument FileDocument type for declarative export - Fix undefined behavior: move Array construction inside withMemoryRebound closure (pointer was escaping its valid scope) - Remove unnecessary ?? on non-optional formattedDuration - Remove dead @State scrollProxy property - Cache DateFormatter as static let; add shared MeetingTranscript.formatTime() utility to eliminate per-call allocation in view and model - Remove dead code: checkScreenRecordingPermission(), openScreenRecordingSettings(), and import ScreenCaptureKit from PermissionManager - Deduplicate SF Symbols: remove private extension, use central SFSymbols.swift, add stopFill as the only genuinely new symbol - Add safety comment on nonisolated(unsafe) let inputBuffer explaining why it is safe (synchronous converter callback)
48d546c to
2cd40bf
Compare
Remove wispr 19.06.04/, wispr 19.06.05.xcodeproj/, wisprTests 19.06.04/, wisprUITests 19.06.04/, and wispr-cli 19.06.04/ — these are local Xcode snapshots that were swept in during the merge conflict resolution.
Hi @sebsto, thank you for the thorough review! I've addressed all the feedback:
Swift 6 Structured Concurrency fixes:
MeetingAudioEngine: Replaced per-bufferTaskspawning withAsyncStreambridge pattern — tap callbacks yield into continuations, single consumer tasks read on the actor. Eliminates hundreds of short-lived tasks and provides natural backpressure. Also replaced@unchecked Sendablewith plainSendableonSystemAudioOutputHandler.MeetingStateManager: Removed all 5 redundantawait MainActor.run {}wrappers (Tasks inherit@MainActorisolation). Replaced 5 unstructuredTaskproperties with a singlerecordingTaskusingwithTaskGroup— cancelling the parent cascades to all children. AddedcancelRecording()for synchronous cleanup.wisprApp.swift: Replaced fire-and-forgetTask { await msm.stopMeeting() }inapplicationWillTerminatewith synchronouscancelRecording().Ready for re-review when you have a chance!