-
Notifications
You must be signed in to change notification settings - Fork 31
feature: copy transcript and dark mode icon #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Will review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements two major features: a copy transcript functionality for copying transcription text to clipboard and system-wide audio recording capabilities. It also includes foundational work for timestamped transcription support and various UI improvements.
- Adds copy transcription button alongside the existing copy summary functionality
- Implements system-wide audio recording via SystemWideTap for capturing all system audio rather than specific applications
- Introduces timestamped transcription models and utilities with WhisperKit integration
Reviewed Changes
Copilot reviewed 44 out of 51 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| cli | New build script for managing Xcode operations |
| SummaryView.swift | Adds copy transcription button to UI |
| SummaryViewModel.swift | Implements copyTranscription() method |
| TranscriptionService.swift | Enhanced with timestamped transcription support |
| SystemWideTap.swift | New system-wide audio capture implementation |
| AudioRecordingCoordinator.swift | Updated to support both process-specific and system-wide recording |
| RecordingInfo.swift | Added timestampedTranscription field |
| SelectableApp.swift | Added "All Apps" option for system-wide recording |
| Assets.xcassets | Added dark mode icon assets |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| do { | ||
| let recording = try self.fetchRecordingEntity(id: id, context: context) | ||
|
|
||
| // Encode the timestamped transcription to binary data |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] This comment is unnecessary as the code clearly shows JSON encoding. Consider removing this comment as it doesn't add value beyond what the code expresses.
| // Encode the timestamped transcription to binary data |
|
|
||
| private func loadModel(_ modelName: String, isDownloaded: Bool) async throws { | ||
| do { | ||
| print("Loading WhisperKit model: \(modelName), isDownloaded: \(isDownloaded)") |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using print() statements for logging in production code is not recommended. These should use the logger instance that's already available in the class for consistent logging behavior.
| } | ||
| ) | ||
|
|
||
| print("WhisperKit model loaded successfully: \(modelName)") |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using print() statements for logging in production code is not recommended. These should use the logger instance that's already available in the class for consistent logging behavior.
| try await whisperModelRepository.markAsDownloaded(name: modelName, sizeInMB: nil) | ||
| let modelInfo = await WhisperKit.getModelSizeInfo(for: modelName) | ||
| try await whisperModelRepository.markAsDownloaded(name: modelName, sizeInMB: Int64(modelInfo.totalSizeMB)) | ||
| print("Model marked as downloaded: \(modelName), size: \(modelInfo.totalSizeMB) MB") |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using print() statements for logging in production code is not recommended. These should use the logger instance that's already available in the class for consistent logging behavior.
|
|
||
| } catch { | ||
| throw TranscriptionError.modelLoadingFailed(error.localizedDescription) | ||
| print("Failed to load WhisperKit model \(modelName): \(error)") |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using print() statements for logging in production code is not recommended. These should use the logger instance that's already available in the class for consistent logging behavior.
| await MainActor.run { | ||
| systemWideTap.activate() | ||
| } |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The systemWideTap.activate() call is redundant here since it's already called earlier in the start() method. This could cause issues or unnecessary work.
| await MainActor.run { | |
| systemWideTap.activate() | |
| } |
| <key>com.apple.security.temporary-exception.audio-unit-host</key> | ||
| <true/> |
Copilot
AI
Sep 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using temporary security exceptions in production should be avoided. This entitlement bypasses sandbox restrictions and may not be acceptable for App Store distribution. Consider implementing proper audio unit hosting within the sandbox.
| <key>com.apple.security.temporary-exception.audio-unit-host</key> | |
| <true/> |
|
Closing - Maybe I will re-propose when the project switches to MIT. Sorry for the annoyance. |
The project is MIT Bro. Check it out |
|
I am deeply grateful, Rawa. But let me rework this a bit because I don't think it's up to quality standards. ^^ (I have learnt a lot about MacOS/Swift during the last week, beyond what my intelligent unanimated friends did for me) |
7a7cfe0 to
1ed4753
Compare
Description
Once the system transcription was working, I attempted to implement a feature I needed: copying the transcription text.
I have also enabled the transcript timestamps (supported by WhisperKit).
The format of the transcript for the moment looks like
I plan to change the format of the transcript text soon to match something like
This PR also contains the icon for dark mode.
Type of Change
Testing
Checklist
Screenshots (if applicable)
Additional Notes
You must resolve #12 before merging this pull request. This code is hereby granted only if this software is licensed with an open-source license.