Conversation
📝 WalkthroughWalkthroughAdds fallback provider support with timeout and automatic retry, history export to text, keychain accessibility migration, analytics tracking for exports and fallbacks, app lifecycle delegation, Groq client prompt handling, and expanded README documentation. Includes UI for export and fallback configuration. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant App
participant Primary as Primary Provider
participant Fallback as Fallback Provider
participant History as History & Analytics
User->>App: Trigger transcription
App->>App: Refresh selected provider & timeout
App->>Primary: Transcribe (with timeout)
alt Primary success
Primary-->>App: Transcription
App->>History: deliver (save, clipboard, notify)
else Primary timeout/error (shouldTryFallback)
Primary--xApp: Error/Timeout
App->>Fallback: Transcribe (fallback)
alt Fallback success
Fallback-->>App: Transcription
App->>History: trackFallbackUsed(primary, fallback, reason)
App->>History: deliver (save, clipboard, notify)
else Fallback fails
Fallback--xApp: Error
App->>App: surfaceError to user
end
end
sequenceDiagram
autonumber
participant User
participant HomeView as Home View
participant HistoryExporter as HistoryExporter
participant FileSystem as File System
participant Analytics as Analytics
User->>HomeView: Click Export
HomeView->>HistoryExporter: exportAndTrack(records)
HistoryExporter->>HistoryExporter: Format records as text
HistoryExporter->>FileSystem: Present NSSavePanel
User->>FileSystem: Choose location & confirm
FileSystem-->>HistoryExporter: URL returned
HistoryExporter->>FileSystem: Write UTF-8 .txt
FileSystem-->>HistoryExporter: Success
HistoryExporter-->>HomeView: Return success
HomeView->>Analytics: trackHistoryExported(count)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dhavnii/Features/Settings/SettingsView.swift (1)
1378-1398:⚠️ Potential issue | 🟡 Minor
resetApp()does not clear the newfallbackTranscriptionProviderandtranscriptionTimeoutSecondsAppStorage keys.When the user resets the app, the fallback provider and timeout settings will persist, which is inconsistent with the "Clear all settings and data" intent.
🛡️ Proposed fix — clear fallback settings on reset
private func resetApp() { UserDefaults.standard.removeObject(forKey: "hasCompletedOnboarding") UserDefaults.standard.removeObject(forKey: "autoLaunchEnabled") UserDefaults.standard.removeObject(forKey: "selectedTranscriptionProvider") + UserDefaults.standard.removeObject(forKey: "fallbackTranscriptionProvider") + UserDefaults.standard.removeObject(forKey: "transcriptionTimeoutSeconds") for provider in TranscriptionProviderType.allCases {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Settings/SettingsView.swift` around lines 1378 - 1398, The resetApp() function currently omits clearing the AppStorage keys for the fallback provider and timeout; update resetApp to remove the UserDefaults/AppStorage entries for "fallbackTranscriptionProvider" and "transcriptionTimeoutSeconds" (or use the same key constants used by the AppStorage wrappers) alongside the other removeObject calls, ensure these removes occur before UserDefaults.standard.synchronize(), and if you maintain corresponding in-memory state variables update them (e.g., clear any fallbackTranscriptionProvider and reset transcription timeout in appState) so the app truly returns to default settings.
🧹 Nitpick comments (5)
dhavnii/Features/Transcription/TranscriptionService.swift (2)
199-217:deliver()is markedthrowsbut never throws.The function signature is
async throwsbut all code paths either return normally or callresetAfterDelay()without throwing. If thethrowsis intentional for future use, consider adding a comment; otherwise, remove it to avoid misleading callers.Proposed fix
- private func deliver(transcription: String, provider: TranscriptionProviderType) async throws { + private func deliver(transcription: String, provider: TranscriptionProviderType) async {And update the call sites on lines 128 and 149:
- try await deliver(transcription: transcription, provider: primary) + await deliver(transcription: transcription, provider: primary)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Transcription/TranscriptionService.swift` around lines 199 - 217, The deliver(transcription:provider:) function is declared async throws but currently never throws; remove the throws from its signature (change to async) and update all call sites (e.g., the invocations referenced around the previous call locations) to no longer use try/try? or try await (or remove redundant do/catch blocks) so callers compile; alternatively, if you intend to keep throws for planned future errors, add a clear comment above deliver explaining why it remains throwing — reference the deliver(transcription:provider:) method and the call sites that currently use try/try await so you can update them accordingly.
239-247:liveSelectedProvider()mutatesselectedProvideras a side effect.This getter-like function silently updates
self.selectedProvideron line 245. While it keeps the instance variable in sync, it's a hidden side effect that could surprise future maintainers. Consider either renaming torefreshSelectedProvider()to signal mutation, or separating the read from the update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Transcription/TranscriptionService.swift` around lines 239 - 247, The method liveSelectedProvider() currently mutates self.selectedProvider as a hidden side effect; either rename it to refreshSelectedProvider() to make the mutation explicit (update the function name and all callers) or remove the mutation so it becomes a pure getter (have liveSelectedProvider() only read UserDefaults and return a TranscriptionProviderType, and create a new explicit mutating method e.g. updateSelectedProviderFromDefaults() that assigns to selectedProvider when state sync is required); update references to selectedProvider, liveSelectedProvider(), and any callers to use the new naming/behavior consistently.dhavnii/Features/History/HistoryExporter.swift (1)
43-72: Consider including a trailing newline in the exported file.The formatted output ends with the last record's text, with no final newline. Many text editors and Unix tools expect a trailing newline. Minor nit.
Proposed fix
- return header + blocks.joined(separator: "\n\n────────────────────────────────────────\n\n") + return header + blocks.joined(separator: "\n\n────────────────────────────────────────\n\n") + "\n"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/History/HistoryExporter.swift` around lines 43 - 72, The formatted(_ records: [TranscriptionRecord]) -> String output lacks a trailing newline; update the function to append a single trailing newline to the returned string (e.g., return (header + blocks.joined(...)) + "\n") so the exported file ends with a newline; ensure you only add one newline regardless of record count.dhavnii/Features/Home/HomeView.swift (1)
338-344: Duplicated export logic — consider extracting a shared helper.This export-then-track pattern is duplicated verbatim in
SettingsView.swift(lines 1497–1501). Consider extracting a shared static method (e.g., onHistoryExporter) that performs the export and conditionally fires the analytics event, so both call sites stay in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Home/HomeView.swift` around lines 338 - 344, The export-then-track logic is duplicated; refactor by adding a shared helper on HistoryExporter (e.g., a static method exportAndTrack(records: [Transcription]) -> Bool) that calls HistoryExporter.exportAsText(records) and, if true, calls AnalyticsManager.shared.trackHistoryExported(count: records.count); then update HomeView.exportHistory() (and the duplicate in SettingsView) to call HistoryExporter.exportAndTrack(viewModel.transcriptions) instead of calling exportAsText and track separately.dhavnii/Features/Settings/SettingsView.swift (1)
1136-1173: Timeout picker is visible even when no fallback is selected.The subtitle reads "Fallback kicks in if primary exceeds this time", which is confusing when the fallback is set to "None". Consider conditionally showing this section only when
!fallbackRaw.isEmpty, or at minimum adjusting the description.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Settings/SettingsView.swift` around lines 1136 - 1173, The timeout picker and its subtitle are shown even when no fallback is selected; wrap the HStack (the timeout UI using timeoutOptions, timeoutSeconds, timeoutLabel) in a conditional that only renders when fallbackRaw is not empty (e.g., if !fallbackRaw.isEmpty) or alternatively change the subtitle text to be conditional based on fallbackRaw (show “Fallback kicks in if primary exceeds this time” when fallback exists and a neutral/helpful string when fallbackRaw is empty). Ensure the condition references the existing symbols timeoutOptions, timeoutSeconds, timeoutLabel and fallbackRaw so the correct view is shown or the description updated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dhavnii/Core/Security/SecureStorage.swift`:
- Around line 128-142: The migration currently marks completion unconditionally
in migrateKeychainAccessibility even though try? storeAPIKey(...) can fail
silently; change this to track success per provider by calling storeAPIKey(for:)
and capturing/propagating errors (or returning Bool) so you can determine
overall success, and only set UserDefaults.standard.set(true, forKey:
migrationKey) if all providers succeeded (or alternatively persist per-provider
migration status); update retrieveAPIKey, storeAPIKey usages in
migrateKeychainAccessibility to handle errors explicitly and use those return
values/errors to decide whether to write the migrationKey flag.
In `@dhavnii/Features/Settings/SettingsView.swift`:
- Around line 1210-1218: In saveFallbackKey(), the call to
SecureStorage.storeAPIKey(apiKeyText, for: type) is using try? which swallows
failures but fallbackHasKey is set to true unconditionally; change to a
do-try-catch (or capture the thrown error) around SecureStorage.storeAPIKey and
only set fallbackHasKey = true, isEditingKey = false and apiKeyText = "" when
the store succeeds; on catch, log or handle the error and do not change
fallbackHasKey (keep isEditingKey true so the UI reflects the failed write).
Ensure you reference the saveFallbackKey function, the fallbackRaw ->
TranscriptionProviderType(rawValue:) conversion, SecureStorage.storeAPIKey, and
the fallbackHasKey/isEditingKey/apiKeyText variables when making the change.
In `@dhavnii/Features/Transcription/GroqAPIClient.swift`:
- Around line 15-16: The systemPrompt string (variable systemPrompt in
GroqAPIClient.swift) is grammatically broken and instructs the model to alter
user speech rather than preserve fidelity; replace it with a clear, grammatical
prompt that only provides style/vocabulary/context hints (e.g., preferred
formatting, domain-specific terms) and removes any instruction to rewrite or
delete utterances, and instead make "cleaned" or "normalized" output an explicit
opt-in behavior exposed via a new flag/parameter (e.g., preserveVerbatim: Bool
or normalizeOutput: Bool) on the Groq API call method so callers choose whether
to request corrections; update any uses of systemPrompt (or rename to
transcriptionPrompt) to reflect the changed semantics and ensure the prompt
aligns with Groq/Whisper guidance (vocabulary/style only).
In `@dhavnii/Features/Transcription/TranscriptionService.swift`:
- Around line 129-157: The fallback logic in TranscriptionService currently
treats any non-TranscriptionError as eligible for fallback by using
(primaryError as? TranscriptionError)?.shouldTryFallback ?? true; change this to
default to false so unknown errors do not trigger retries (e.g., use ?? false)
and explicitly exclude CancellationError (or Task.isCancelled) from attempting
the fallback; update the eligibleForFallback computation in the catch block
around primaryError and keep the rest of the fallback flow (attempt(provider:),
deliver(...), surfaceError(...)) unchanged.
In `@README.md`:
- Around line 81-112: The fenced code blocks in README.md (the project tree
block containing lines like "dhavnii/" and the environment variable block
containing "GROQ_API_KEY=<your key>") are missing language identifiers; update
both fenced code blocks to include a language (e.g., add ```text before the
project structure block that starts with "dhavnii/" and add ```text before the
environment variable block that contains "GROQ_API_KEY=<your key>") so they
conform to markdownlint MD040.
- Around line 132-135: Update the README git clone example to use the real
repository path instead of the placeholder: replace "git clone
https://github.com/<your-org>/dhavnii.git" with the actual repo URL "git clone
https://github.com/maker-or/openwispher.git" so the clone command in the README
(the bash snippet) points to maker-or/openwispher.
---
Outside diff comments:
In `@dhavnii/Features/Settings/SettingsView.swift`:
- Around line 1378-1398: The resetApp() function currently omits clearing the
AppStorage keys for the fallback provider and timeout; update resetApp to remove
the UserDefaults/AppStorage entries for "fallbackTranscriptionProvider" and
"transcriptionTimeoutSeconds" (or use the same key constants used by the
AppStorage wrappers) alongside the other removeObject calls, ensure these
removes occur before UserDefaults.standard.synchronize(), and if you maintain
corresponding in-memory state variables update them (e.g., clear any
fallbackTranscriptionProvider and reset transcription timeout in appState) so
the app truly returns to default settings.
---
Nitpick comments:
In `@dhavnii/Features/History/HistoryExporter.swift`:
- Around line 43-72: The formatted(_ records: [TranscriptionRecord]) -> String
output lacks a trailing newline; update the function to append a single trailing
newline to the returned string (e.g., return (header + blocks.joined(...)) +
"\n") so the exported file ends with a newline; ensure you only add one newline
regardless of record count.
In `@dhavnii/Features/Home/HomeView.swift`:
- Around line 338-344: The export-then-track logic is duplicated; refactor by
adding a shared helper on HistoryExporter (e.g., a static method
exportAndTrack(records: [Transcription]) -> Bool) that calls
HistoryExporter.exportAsText(records) and, if true, calls
AnalyticsManager.shared.trackHistoryExported(count: records.count); then update
HomeView.exportHistory() (and the duplicate in SettingsView) to call
HistoryExporter.exportAndTrack(viewModel.transcriptions) instead of calling
exportAsText and track separately.
In `@dhavnii/Features/Settings/SettingsView.swift`:
- Around line 1136-1173: The timeout picker and its subtitle are shown even when
no fallback is selected; wrap the HStack (the timeout UI using timeoutOptions,
timeoutSeconds, timeoutLabel) in a conditional that only renders when
fallbackRaw is not empty (e.g., if !fallbackRaw.isEmpty) or alternatively change
the subtitle text to be conditional based on fallbackRaw (show “Fallback kicks
in if primary exceeds this time” when fallback exists and a neutral/helpful
string when fallbackRaw is empty). Ensure the condition references the existing
symbols timeoutOptions, timeoutSeconds, timeoutLabel and fallbackRaw so the
correct view is shown or the description updated.
In `@dhavnii/Features/Transcription/TranscriptionService.swift`:
- Around line 199-217: The deliver(transcription:provider:) function is declared
async throws but currently never throws; remove the throws from its signature
(change to async) and update all call sites (e.g., the invocations referenced
around the previous call locations) to no longer use try/try? or try await (or
remove redundant do/catch blocks) so callers compile; alternatively, if you
intend to keep throws for planned future errors, add a clear comment above
deliver explaining why it remains throwing — reference the
deliver(transcription:provider:) method and the call sites that currently use
try/try await so you can update them accordingly.
- Around line 239-247: The method liveSelectedProvider() currently mutates
self.selectedProvider as a hidden side effect; either rename it to
refreshSelectedProvider() to make the mutation explicit (update the function
name and all callers) or remove the mutation so it becomes a pure getter (have
liveSelectedProvider() only read UserDefaults and return a
TranscriptionProviderType, and create a new explicit mutating method e.g.
updateSelectedProviderFromDefaults() that assigns to selectedProvider when state
sync is required); update references to selectedProvider,
liveSelectedProvider(), and any callers to use the new naming/behavior
consistently.
Comment on lines
+129
to
+157
| } catch let primaryError { | ||
| print("⚠️ Primary provider \(primary.rawValue) failed: \(primaryError)") | ||
|
|
||
| // Determine whether to try the fallback | ||
| let eligibleForFallback = (primaryError as? TranscriptionError)?.shouldTryFallback ?? true | ||
|
|
||
| if eligibleForFallback, let fallback, fallback != primary { | ||
| print("🔄 Trying fallback provider: \(fallback.rawValue)") | ||
| do { | ||
| let transcription = try await attempt( | ||
| provider: fallback, | ||
| audioData: audioData, | ||
| timeoutSeconds: timeoutSeconds | ||
| ) | ||
| // Successful fallback — track analytics silently, no user-facing notice | ||
| AnalyticsManager.shared.trackFallbackUsed( | ||
| primary: primary, | ||
| fallback: fallback, | ||
| reason: (primaryError as? TranscriptionError)?.analyticsReason ?? "unknown" | ||
| ) | ||
| try await deliver(transcription: transcription, provider: fallback) | ||
| } catch let fallbackError { | ||
| print("❌ Fallback provider \(fallback.rawValue) also failed: \(fallbackError)") | ||
| surfaceError(fallbackError) | ||
| } | ||
| } else { | ||
| surfaceError(primaryError) | ||
| } | ||
| } |
There was a problem hiding this comment.
?? true default may trigger fallback on unexpected error types (e.g., CancellationError).
Line 133: if the primary error is not a TranscriptionError (e.g., CancellationError from structured concurrency), the cast fails and ?? true causes a fallback attempt. This could be surprising — for instance, task cancellation should not trigger a retry with a different provider.
Consider defaulting to false so only known transient errors trigger fallback:
Proposed fix
- let eligibleForFallback = (primaryError as? TranscriptionError)?.shouldTryFallback ?? true
+ let eligibleForFallback = (primaryError as? TranscriptionError)?.shouldTryFallback ?? false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dhavnii/Features/Transcription/TranscriptionService.swift` around lines 129 -
157, The fallback logic in TranscriptionService currently treats any
non-TranscriptionError as eligible for fallback by using (primaryError as?
TranscriptionError)?.shouldTryFallback ?? true; change this to default to false
so unknown errors do not trigger retries (e.g., use ?? false) and explicitly
exclude CancellationError (or Task.isCancelled) from attempting the fallback;
update the eligibleForFallback computation in the catch block around
primaryError and keep the rest of the fallback flow (attempt(provider:),
deliver(...), surfaceError(...)) unchanged.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
dhavnii/Features/Home/HomeView.swift (1)
253-269:contentHeaderis dead code — never referenced inbody.
HomeContentView.bodyonly rendersexportToolbar,emptyStateView, ortranscriptionsList.contentHeaderis declared but never used.♻️ Proposed fix — remove unused property
- // MARK: - Header - - private var contentHeader: some View { - HStack { - VStack(alignment: .leading, spacing: 4) { - Text("Home") - .font(.title2) - .fontWeight(.semibold) - .foregroundStyle(.primary) - .font(.subheadline) - .foregroundStyle(.secondary) - } - - Spacer() - } - .padding(.horizontal, 24) - .padding(.vertical, 16) - .background(.ultraThinMaterial.opacity(0.5)) - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Home/HomeView.swift` around lines 253 - 269, The private computed view property contentHeader is dead code (never referenced by HomeContentView.body which only uses exportToolbar, emptyStateView, and transcriptionsList); remove the unused contentHeader declaration from HomeView.swift to clean up unused code, or if you intended to show a header instead of removing it, insert contentHeader into HomeContentView.body in the appropriate place alongside exportToolbar/transcriptionsList/emptyStateView; reference the contentHeader symbol and HomeContentView.body when making the change.dhavnii/Features/Transcription/TranscriptionService.swift (1)
195-197: Replace force-unwrap with aguard let.With two tasks always added to the group,
group.next()will not returnnilon the first call, so the!is technically safe — but it is still a code smell that can cause a silent crash if the group composition ever changes.♻️ Proposed fix
- let result = try await group.next()! - group.cancelAll() - return result + guard let result = try await group.next() else { + group.cancelAll() + throw TranscriptionError.timeout(provider: providerType.rawValue) + } + group.cancelAll() + return result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dhavnii/Features/Transcription/TranscriptionService.swift` around lines 195 - 197, Replace the force-unwrap of group.next() with a safe optional unwrap: call let resultOptional = try await group.next(); use guard let result = resultOptional else { handle the unexpected nil (e.g., throw a descriptive error or return a default/failure) } then call group.cancelAll() and return result; update the surrounding function (TranscriptionService method using the TaskGroup) to propagate the thrown error if you choose to throw so callers can handle the unexpected nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dhavnii/Features/History/HistoryExporter.swift`:
- Line 10: The import declaration in HistoryExporter.swift uses the Swift 6-only
access modifier token "internal import UniformTypeIdentifiers"; remove the
access modifier and replace it with a plain import by deleting the "internal"
keyword so the file simply uses "import UniformTypeIdentifiers" (locate the line
containing "internal import UniformTypeIdentifiers" to change).
In `@dhavnii/Features/Settings/SettingsView.swift`:
- Around line 1227-1231: removeFallbackKey() currently swallows
SecureStorage.deleteAPIKey errors via try? and unconditionally sets
fallbackHasKey = false; change it to attempt deletion with do/catch (or
propagate the thrown error) using SecureStorage.deleteAPIKey(for: type) inside
removeFallbackKey(), only set fallbackHasKey = false on successful deletion, and
handle/log or surface the caught KeychainError (e.g., show an alert or
processLogger.error) so the UI remains consistent; ensure you still guard the
TranscriptionProviderType(rawValue: fallbackRaw) check and reference the same
symbols (removeFallbackKey, fallbackRaw, TranscriptionProviderType,
SecureStorage.deleteAPIKey, fallbackHasKey).
- Around line 1133-1134: The Divider() is rendered unconditionally causing a
stray separator when fallbackRaw is empty; wrap the Divider() in the same
conditional used for the API-key and timeout sections (the guard on fallbackRaw,
e.g., the if !fallbackRaw.isEmpty check) so the divider is only shown when those
sections are present (place the Divider() inside the block that contains the
API-key/timeout UI associated with fallbackRaw).
In `@README.md`:
- Around line 133-135: The README contains an incorrect directory change
command: after the git clone command `git clone
https://github.com/maker-or/openwispher.git` the subsequent `cd dhavnii` is
wrong; update that command to `cd openwispher` so it matches the cloned
repository name and allows subsequent steps to run in the correct project
directory.
---
Nitpick comments:
In `@dhavnii/Features/Home/HomeView.swift`:
- Around line 253-269: The private computed view property contentHeader is dead
code (never referenced by HomeContentView.body which only uses exportToolbar,
emptyStateView, and transcriptionsList); remove the unused contentHeader
declaration from HomeView.swift to clean up unused code, or if you intended to
show a header instead of removing it, insert contentHeader into
HomeContentView.body in the appropriate place alongside
exportToolbar/transcriptionsList/emptyStateView; reference the contentHeader
symbol and HomeContentView.body when making the change.
In `@dhavnii/Features/Transcription/TranscriptionService.swift`:
- Around line 195-197: Replace the force-unwrap of group.next() with a safe
optional unwrap: call let resultOptional = try await group.next(); use guard let
result = resultOptional else { handle the unexpected nil (e.g., throw a
descriptive error or return a default/failure) } then call group.cancelAll() and
return result; update the surrounding function (TranscriptionService method
using the TaskGroup) to propagate the thrown error if you choose to throw so
callers can handle the unexpected nil.
|
|
||
| import AppKit | ||
| import Foundation | ||
| internal import UniformTypeIdentifiers |
There was a problem hiding this comment.
internal import requires Swift 6.0 (or the AccessLevelOnImport experimental flag on Swift 5.x).
SE-0409 adds access-level modifiers on import declarations and is available from Swift 6.0. In Swift 5, this feature is still behind the experimental flag AccessLevelOnImport, which is not enabled by default. Since the README targets Swift 5.9+ (Xcode 15), this line will fail to compile unless the project has adopted Swift 6 language mode or explicitly enabled the experimental flag. A plain import UniformTypeIdentifiers is fully equivalent here (a single-module app target gains nothing from access-controlling the import).
🐛 Proposed fix
-internal import UniformTypeIdentifiers
+import UniformTypeIdentifiers📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| internal import UniformTypeIdentifiers | |
| import UniformTypeIdentifiers |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dhavnii/Features/History/HistoryExporter.swift` at line 10, The import
declaration in HistoryExporter.swift uses the Swift 6-only access modifier token
"internal import UniformTypeIdentifiers"; remove the access modifier and replace
it with a plain import by deleting the "internal" keyword so the file simply
uses "import UniformTypeIdentifiers" (locate the line containing "internal
import UniformTypeIdentifiers" to change).
Comment on lines
+1133
to
+1134
| Divider() | ||
| .padding(.horizontal, 16) |
There was a problem hiding this comment.
Orphaned Divider visible when no fallback provider is configured.
The Divider() at line 1133 is rendered unconditionally. When fallbackRaw is empty, the API-key section (guarded by if !fallbackRaw.isEmpty at line 1035) and the timeout section (guarded by if !fallbackRaw.isEmpty at line 1137) are both hidden, leaving this divider at the bottom of the card with nothing below it — a stray separator.
🐛 Proposed fix — wrap the divider in the same guard
+ if !fallbackRaw.isEmpty {
Divider()
.padding(.horizontal, 16)
-
- // Timeout threshold — only relevant when a fallback provider is selected
- if !fallbackRaw.isEmpty {
HStack { /* timeout row */ }
.padding(16)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Divider() | |
| .padding(.horizontal, 16) | |
| if !fallbackRaw.isEmpty { | |
| Divider() | |
| .padding(.horizontal, 16) | |
| HStack { /* timeout row */ } | |
| .padding(16) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dhavnii/Features/Settings/SettingsView.swift` around lines 1133 - 1134, The
Divider() is rendered unconditionally causing a stray separator when fallbackRaw
is empty; wrap the Divider() in the same conditional used for the API-key and
timeout sections (the guard on fallbackRaw, e.g., the if !fallbackRaw.isEmpty
check) so the divider is only shown when those sections are present (place the
Divider() inside the block that contains the API-key/timeout UI associated with
fallbackRaw).
Comment on lines
+1227
to
+1231
| private func removeFallbackKey() { | ||
| guard let type = TranscriptionProviderType(rawValue: fallbackRaw) else { return } | ||
| try? SecureStorage.deleteAPIKey(for: type) | ||
| fallbackHasKey = false | ||
| } |
There was a problem hiding this comment.
removeFallbackKey() ignores delete errors and unconditionally marks key absent.
try? silently discards any KeychainError, but fallbackHasKey = false is set regardless. If the delete fails, the UI shows "no key configured" while the key remains in the keychain — the same class of inconsistency that was fixed in saveFallbackKey().
🐛 Proposed fix
private func removeFallbackKey() {
guard let type = TranscriptionProviderType(rawValue: fallbackRaw) else { return }
- try? SecureStorage.deleteAPIKey(for: type)
- fallbackHasKey = false
+ do {
+ try SecureStorage.deleteAPIKey(for: type)
+ fallbackHasKey = false
+ } catch {
+ print("❌ FallbackSettingsCard: failed to remove API key for \(type.rawValue): \(error)")
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dhavnii/Features/Settings/SettingsView.swift` around lines 1227 - 1231,
removeFallbackKey() currently swallows SecureStorage.deleteAPIKey errors via
try? and unconditionally sets fallbackHasKey = false; change it to attempt
deletion with do/catch (or propagate the thrown error) using
SecureStorage.deleteAPIKey(for: type) inside removeFallbackKey(), only set
fallbackHasKey = false on successful deletion, and handle/log or surface the
caught KeychainError (e.g., show an alert or processLogger.error) so the UI
remains consistent; ensure you still guard the
TranscriptionProviderType(rawValue: fallbackRaw) check and reference the same
symbols (removeFallbackKey, fallbackRaw, TranscriptionProviderType,
SecureStorage.deleteAPIKey, fallbackHasKey).
Comment on lines
+133
to
+135
| git clone https://github.com/maker-or/openwispher.git | ||
| cd dhavnii | ||
| ``` |
There was a problem hiding this comment.
cd dhavnii should be cd openwispher after the clone.
The clone command (now correctly pointing to maker-or/openwispher.git) creates a directory named openwispher, but the next line still tries to cd dhavnii.
📝 Proposed fix
git clone https://github.com/maker-or/openwispher.git
-cd dhavnii
+cd openwispher📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| git clone https://github.com/maker-or/openwispher.git | |
| cd dhavnii | |
| ``` | |
| git clone https://github.com/maker-or/openwispher.git | |
| cd openwispher |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 133 - 135, The README contains an incorrect directory
change command: after the git clone command `git clone
https://github.com/maker-or/openwispher.git` the subsequent `cd dhavnii` is
wrong; update that command to `cd openwispher` so it matches the cloned
repository name and allows subsequent steps to run in the correct project
directory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation