feat: Transcribe Again with Clean Pipeline Reconstruction#227
feat: Transcribe Again with Clean Pipeline Reconstruction#227pianoandglass wants to merge 7 commits into
Conversation
- Isolates 'Transcribe Again' logic into dedicated extensions (AppState+TranscribeAgain, PostProcessingService+Retry). - Always re-transcribes the raw audio for retry requests. - Implements prompt replay via SQLite parsing and dynamic text injection. - Ensures strict Swift 6 concurrency compliance using @mainactor and detached Task groups. - Preserves raw transcripts and correctly restores clipboard state if needed.
📝 WalkthroughWalkthroughAdds a "Transcribe Again" retry pipeline to ChangesTranscribe Again Retry Flow
Sequence Diagram(s)sequenceDiagram
participant User
participant MenuBarView
participant AppState
participant executeRetry
participant PostProcessingService
participant pipelineHistoryStore
participant applyRetryOutput
User->>MenuBarView: click "Transcribe Again"
MenuBarView->>AppState: retryTranscription(item, overrideModel, action)
AppState->>AppState: add item.id to retryingItemIDs
AppState->>executeRetry: run async pipeline
executeRetry->>PostProcessingService: postProcess or commandTransform
PostProcessingService-->>executeRetry: processed result
executeRetry-->>AppState: success or failure
AppState->>pipelineHistoryStore: update item (success or error-marked)
AppState->>applyRetryOutput: dispatch output action
applyRetryOutput->>User: paste at cursor or copy to clipboard
AppState->>AppState: remove item.id from retryingItemIDs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🧹 Nitpick comments (5)
Sources/PostProcessingService+Retry.swift (3)
5-101: ⚖️ Poor tradeoffConsider extracting shared request/response logic.
retryWithPromptduplicates significant portions ofprocessandprocessCommandTransform(request setup, payload construction, network call, response parsing, sanitization). If a bug is fixed or a pattern is updated in one method, it could be missed in the retry path.Consider extracting a shared helper like
executeAPIRequest(url:payload:config:)that handles the network call and response parsing, with caller-specific logic for prompt construction and sanitization remaining in each method. However, given the isolated nature of the retry flow, the current duplication is manageable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/PostProcessingService`+Retry.swift around lines 5 - 101, The retryWithPrompt implementation duplicates request setup, payload construction, network call and response parsing found in process and processCommandTransform; extract a shared helper (e.g. executeAPIRequest(url:payload:config:)) that accepts the URL, JSON payload and ModelConfiguration and returns the parsed content string (or throws PostProcessingError) using LLMAPITransport and the existing HTTP/JSON checks; keep caller-specific logic (building promptForDisplay, choosing system/user messages, and sanitization via sanitizeCommandModeTranscript / sanitizePostProcessedTranscript) in retryWithPrompt, process and processCommandTransform, and update those methods to call the new helper to remove duplication while still using ModelConfiguration.stripThinkTags when needed.
11-20: ⚡ Quick winMissing task group timeout wrapper for robust async cancellation.
The main
postProcessandcommandTransformmethods usewithThrowingTaskGroupto race the API request against a timeout task (lines 162-191 and 212-241 in PostProcessingService.swift). This ensures all async work is cancelled on timeout.retryWithPromptonly setsrequest.timeoutInterval, which doesn't cancel other async operations that might occur during the call.⏱️ Recommended timeout pattern
Wrap the request logic in a task group similar to the existing methods:
) async throws -> PostProcessingResult { + let timeoutSeconds = UserDefaults.standard.double(forKey: "post_processing_timeout_seconds") + let effectiveTimeout = timeoutSeconds > 0 ? timeoutSeconds : 20 + + return try await withThrowingTaskGroup(of: PostProcessingResult.self) { group in + group.addTask { + try await self.executeRetryRequest( + systemPrompt: systemPrompt, + userMessage: userMessage, + model: model, + isCommandMode: isCommandMode, + timeoutInterval: effectiveTimeout + ) + } + + group.addTask { + try await Task.sleep(nanoseconds: UInt64(effectiveTimeout * 1_000_000_000)) + throw PostProcessingError.requestTimedOut(effectiveTimeout) + } + + guard let result = try await group.next() else { + throw PostProcessingError.invalidResponse("No post-processing result") + } + group.cancelAll() + return result + } + } + + private func executeRetryRequest( + systemPrompt: String, + userMessage: String, + model: String, + isCommandMode: Bool, + timeoutInterval: TimeInterval + ) async throws -> PostProcessingResult { guard let url = URL(string: "\(baseURL)/chat/completions") else { throw PostProcessingError.invalidResponse("Invalid API base URL: \(baseURL)") } var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("Bearer \(apiKey)", forHTTPHeaderField: "Authorization") request.setValue("application/json", forHTTPHeaderField: "Content-Type") - - let timeoutSeconds = UserDefaults.standard.double(forKey: "post_processing_timeout_seconds") - request.timeoutInterval = timeoutSeconds > 0 ? timeoutSeconds : 20 + request.timeoutInterval = timeoutInterval🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/PostProcessingService`+Retry.swift around lines 11 - 20, The retryWithPrompt implementation only sets request.timeoutInterval but must mirror the other methods by racing the network call against a timeout task so all async work is cancelled on timeout; modify retryWithPrompt (in PostProcessingService+Retry.swift) to wrap the existing request creation and URLSession call inside withThrowingTaskGroup, add one child task that performs the URLSession.data(for:request:) work and another child task that sleeps for timeoutSeconds then throws PostProcessingError.timeout (or the same timeout error type used in postProcess/commandTransform), await the first result and call cancelAll() on the group on completion/error so any remaining tasks are cancelled, and remove reliance on only request.timeoutInterval for enforcing the overall operation timeout.
47-62: ⚡ Quick winHardcoded model configuration creates maintenance burden.
Lines 50-62 duplicate model-specific fallback logic for
"openai/gpt-oss-20b"that already exists in the main service (lines 428-440). IfModelConfigurationis updated for this model or if the default model changes, both locations must be updated.♻️ Proposed refactor to use shared constants
Option 1: Reference the service's default model constant:
let config = ModelConfiguration.config(for: model) if let maxTokens = config.maxCompletionTokens { payload["max_completion_tokens"] = maxTokens - } else if model == "openai/gpt-oss-20b" { - payload["max_completion_tokens"] = 4096 + } else if model == defaultModel { + payload["max_completion_tokens"] = postProcessingMaxCompletionTokens } if let effort = config.reasoningEffort { payload["reasoning_effort"] = effort - } else if model == "openai/gpt-oss-20b" { - payload["reasoning_effort"] = "low" + } else if model == defaultModel { + payload["reasoning_effort"] = defaultModelReasoningEffort } if let include = config.includeReasoning { payload["include_reasoning"] = include - } else if model == "openai/gpt-oss-20b" { - payload["include_reasoning"] = false + } else if model == defaultModel { + payload["include_reasoning"] = false }This requires exposing
defaultModel,postProcessingMaxCompletionTokens, anddefaultModelReasoningEffort(similar to howapiKeyandbaseURLwere exposed).Option 2: Extract a shared helper method in PostProcessingService that both paths can call.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/PostProcessingService`+Retry.swift around lines 47 - 62, Remove the hardcoded "openai/gpt-oss-20b" fallbacks in this block and use the shared model defaults instead: either reference the service-level constants (e.g. PostProcessingService.defaultModel, PostProcessingService.postProcessingMaxCompletionTokens, PostProcessingService.defaultModelReasoningEffort) when deciding payload["max_completion_tokens"], payload["reasoning_effort"], and payload["include_reasoning"], or factor the logic into a shared helper (e.g. PostProcessingService.applyModelFallbacks(to: payload, model: String) or ModelConfiguration.applyFallbacks(to: &payload, for: model)) and call that from here; keep calls to ModelConfiguration.config(for:) for explicit settings and only consult the shared constants/helper for fallbacks so updates happen in one place.Sources/AppState+TranscribeAgain.swift (2)
334-366: 💤 Low valueEmpty catch block silently discards persistence errors.
If
pipelineHistoryStore.update()fails during error finalization, the failure is silently ignored. While this is already an error path, logging would help diagnose issues where the history doesn't reflect retry failures.♻️ Suggested change
do { try pipelineHistoryStore.update(updatedItem) pipelineHistory = pipelineHistoryStore.loadAllHistory() - } catch {} + } catch { + // Best-effort: log but don't overwrite the original error message + }Or log the secondary failure without overwriting
errorMessage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/AppState`+TranscribeAgain.swift around lines 334 - 366, The catch block in finalizeRetryFailure silently discards errors from pipelineHistoryStore.update(), so change it to log the secondary failure instead of ignoring it: inside finalizeRetryFailure, catch errors from pipelineHistoryStore.update(updatedItem) and call an appropriate logger (or append to errorMessage without overwriting the original retry error) mentioning pipelineHistoryStore.update failure and the caught Error, ensuring retryingItemIDs.remove(item.id) still runs; reference finalizeRetryFailure, pipelineHistoryStore.update(updatedItem), pipelineHistory = pipelineHistoryStore.loadAllHistory(), and errorMessage when adding the log/update.
85-168: 💤 Low valueConsider using
os_loginstead ofThe codebase uses
os_logfor logging throughout (seerecordingLogin AppState.swift). Using♻️ Suggested change
} catch { // If fast-path fails (e.g., LLM timeout), fall back to slow-path full context reconstruction. - print("Prompt replay fast-path failed: \(error). Falling back to full pipeline reconstruction.") + // Fast-path failed — falling back to full pipeline reconstruction. }Alternatively, import
os.logand useos_logif this diagnostic is valuable for production debugging.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/AppState`+TranscribeAgain.swift around lines 85 - 168, In executeRetry (inside AppState extension) replace the debug print("Prompt replay fast-path failed: \(error)...") with the project's structured logger (e.g., recordingLog.error or an os_log call) and include the error details; ensure you import os.log if using os_log or reuse the existing recordingLog instance used elsewhere in AppState so the failure message and error are recorded consistently in production logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/MenuBarView.swift`:
- Around line 141-158: The menu currently allows retries whenever lastItem
exists but doesn't check lastItem.canRetry; update the logic so that isRetrying
uses lastItem.map { appState.retryingItemIDs.contains($0.id) } ?? false but also
compute a canRetry boolean from lastItem?.canRetry (e.g., let canRetry =
lastItem?.canRetry ?? false) and use that to disable the Menu
(.disabled(lastItem == nil || !canRetry || isRetrying)); also guard inside the
Button actions (where you call
appState.retryTranscription(item:overrideModel:action:)) to ensure the
item.canRetry is true before calling retryTranscription. Ensure the Menu label
and state reflect canRetry to prevent attempts on items lacking audio/raw
transcript/prompt.
---
Nitpick comments:
In `@Sources/AppState`+TranscribeAgain.swift:
- Around line 334-366: The catch block in finalizeRetryFailure silently discards
errors from pipelineHistoryStore.update(), so change it to log the secondary
failure instead of ignoring it: inside finalizeRetryFailure, catch errors from
pipelineHistoryStore.update(updatedItem) and call an appropriate logger (or
append to errorMessage without overwriting the original retry error) mentioning
pipelineHistoryStore.update failure and the caught Error, ensuring
retryingItemIDs.remove(item.id) still runs; reference finalizeRetryFailure,
pipelineHistoryStore.update(updatedItem), pipelineHistory =
pipelineHistoryStore.loadAllHistory(), and errorMessage when adding the
log/update.
- Around line 85-168: In executeRetry (inside AppState extension) replace the
debug print("Prompt replay fast-path failed: \(error)...") with the project's
structured logger (e.g., recordingLog.error or an os_log call) and include the
error details; ensure you import os.log if using os_log or reuse the existing
recordingLog instance used elsewhere in AppState so the failure message and
error are recorded consistently in production logs.
In `@Sources/PostProcessingService`+Retry.swift:
- Around line 5-101: The retryWithPrompt implementation duplicates request
setup, payload construction, network call and response parsing found in process
and processCommandTransform; extract a shared helper (e.g.
executeAPIRequest(url:payload:config:)) that accepts the URL, JSON payload and
ModelConfiguration and returns the parsed content string (or throws
PostProcessingError) using LLMAPITransport and the existing HTTP/JSON checks;
keep caller-specific logic (building promptForDisplay, choosing system/user
messages, and sanitization via sanitizeCommandModeTranscript /
sanitizePostProcessedTranscript) in retryWithPrompt, process and
processCommandTransform, and update those methods to call the new helper to
remove duplication while still using ModelConfiguration.stripThinkTags when
needed.
- Around line 11-20: The retryWithPrompt implementation only sets
request.timeoutInterval but must mirror the other methods by racing the network
call against a timeout task so all async work is cancelled on timeout; modify
retryWithPrompt (in PostProcessingService+Retry.swift) to wrap the existing
request creation and URLSession call inside withThrowingTaskGroup, add one child
task that performs the URLSession.data(for:request:) work and another child task
that sleeps for timeoutSeconds then throws PostProcessingError.timeout (or the
same timeout error type used in postProcess/commandTransform), await the first
result and call cancelAll() on the group on completion/error so any remaining
tasks are cancelled, and remove reliance on only request.timeoutInterval for
enforcing the overall operation timeout.
- Around line 47-62: Remove the hardcoded "openai/gpt-oss-20b" fallbacks in this
block and use the shared model defaults instead: either reference the
service-level constants (e.g. PostProcessingService.defaultModel,
PostProcessingService.postProcessingMaxCompletionTokens,
PostProcessingService.defaultModelReasoningEffort) when deciding
payload["max_completion_tokens"], payload["reasoning_effort"], and
payload["include_reasoning"], or factor the logic into a shared helper (e.g.
PostProcessingService.applyModelFallbacks(to: payload, model: String) or
ModelConfiguration.applyFallbacks(to: &payload, for: model)) and call that from
here; keep calls to ModelConfiguration.config(for:) for explicit settings and
only consult the shared constants/helper for fallbacks so updates happen in one
place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b10f971-bf8d-4a48-9260-a5bb3e9dafee
📒 Files selected for processing (7)
Sources/AppState+TranscribeAgain.swiftSources/AppState.swiftSources/MenuBarView.swiftSources/PipelineHistoryItem.swiftSources/PostProcessingService+Retry.swiftSources/PostProcessingService.swiftSources/SettingsView.swift
Expose and utilize executeAPIRequest and applyModelConfigAndFallbacks helpers in PostProcessingService to remove duplication; implement task group-based timeout for retryWithPrompt; add canRetry checks in MenuBarView; log failures with os_log using shared recordingLog.
Restore private let recordingLog in AppState.swift and define a static retryLog on AppState to be shared with AppState+TranscribeAgain.swift.
Correct parameter type from ModelConfiguration to ModelConfig for executeAPIRequest and applyModelConfigAndFallbacks to match return type of ModelConfiguration.config(for:).
|
I have addressed all CodeRabbit AI recommendations for this PR. Specifically, I added |
pianoandglass
changed the title
|
@coderabbitai I have completely refactored the retry execution logic to cleanly reconstruct the original AppContext and run post-processing via the standard pipeline service. The brittle regex prompt parsing has been removed, making the retry robust to future prompt format changes, as requested. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cat Sources/AppState+TranscribeAgain.swiftLength of output: 12948
The new approach is clean and correct:
This is exactly what was needed — the retry is now a true pipeline replay, not a brittle prompt reconstruction. Great work on the refactor! 🐇✨ |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/PipelineHistoryItem.swift (1)
84-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
canRetryis broader than the executor contract and enables guaranteed-fail retries.On Line 85,
canRetryreturnstruefor prompt-only entries, butresolveRawTranscriptinSources/AppState+TranscribeAgain.swift(Line 204 onward) only accepts audio or raw transcript. That means a prompt-only history item is shown as retriable and then fails withnoRawTranscriptAndNoAudio.Suggested fix
var canRetry: Bool { - if let prompt = postProcessingPrompt, !prompt.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { - return true - } if !rawTranscript.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { return true } - return audioFileName != nil + if let audioFileName { + return !audioFileName.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + } + return false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/PipelineHistoryItem.swift` around lines 84 - 92, The `canRetry` computed property in `PipelineHistoryItem.swift` returns `true` when a prompt-only entry exists, but the `resolveRawTranscript` method in `AppState+TranscribeAgain.swift` requires either audio or raw transcript to actually perform a retry. Remove the `postProcessingPrompt` check from the `canRetry` property (the first if block that checks for prompt) so that `canRetry` only returns `true` when there is either a non-empty raw transcript or an audio file name present, aligning the property behavior with the actual executor contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/PipelineHistoryItem.swift`:
- Around line 84-92: The `canRetry` computed property in
`PipelineHistoryItem.swift` returns `true` when a prompt-only entry exists, but
the `resolveRawTranscript` method in `AppState+TranscribeAgain.swift` requires
either audio or raw transcript to actually perform a retry. Remove the
`postProcessingPrompt` check from the `canRetry` property (the first if block
that checks for prompt) so that `canRetry` only returns `true` when there is
either a non-empty raw transcript or an audio file name present, aligning the
property behavior with the actual executor contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ccd0653b-196c-476d-a47c-48fde6a31f1d
📒 Files selected for processing (2)
Sources/AppState+TranscribeAgain.swiftSources/PipelineHistoryItem.swift
This comment was marked as outdated.
This comment was marked as outdated.
|
Tip For best results, initiate chat on the files or code changes.
|
1 participant
Refactored PR to drop the brittle Regex parser; now reliably reconstructs AppContext and runs the standard post‑processing pipeline, improving robustness to future code changes.
How it works
Audio transcription is occasionally imperfect (e.g., due to background noise or accent variations), or the subsequent LLM post-processing may fail to yield the desired result using the default model at that exact moment.
This pull request introduces a quick, elegant way to re-transcribe and re-process the last dictation:
Screenshots & Videos
Gravacao.de.Tela.2026-06-02.as.19.56.04.mov
##Core Mechanism
1. Re-processing of Audio (Audio Retransmission)
Whenever a user requests a retry, the system always re-transcribes the raw
.wavaudio using the transcription model. This ensures that if the user changes the transcription settings, or if the original audio file holds a cleaner signal that could benefit from a second pass, the re-processing works directly with a fresh raw text extraction.2. Clean AppContext Reconstruction
Instead of using brittle regex to parse the old prompt, the system now:
AppContextusing the originalcontextAppName,contextWindowTitle,contextSummary,precedingText, andfollowingText.processTranscriptpipeline.This design has a major benefit:
Changes Made
AppState+TranscribeAgain.swift[NEW]AppContextand runsprocessTranscript.SettingsView.swiftMenuBarView.swiftTesting Instructions
make run.Summary by CodeRabbit
New Features