Meeting: harden realtime fallback and async jobs completion#39
Open
rmarinsky wants to merge 9 commits into
Open
Meeting: harden realtime fallback and async jobs completion#39rmarinsky wants to merge 9 commits into
rmarinsky wants to merge 9 commits into
Conversation
…tial stopMeetingRecording previously used any non-empty realtime transcript. For longer meetings that stop before Soniox emits an explicit finished frame, this could surface a truncated transcript instead of the complete async result. Add shouldAcceptRealtimeTranscript: accept the realtime text when it was explicitly finalized, or for short (<30s) recordings; otherwise require a substantial transcript (>=120 chars) before trusting an unfinalized one, falling back to the async jobs pipeline when in doubt. Log when a partial transcript is discarded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds in-progress recording manifests/store, chunked capture and stitcher, sleep flush coordination, push-to-talk hold-start delay (settings/UI/service), AppDelegate cleanup for stop/cancel, model updates, cloud usage mapping, tests, and project build wiring. ChangesRecording pipeline and controls
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
Poem
🚥 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 docstrings
🧪 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Diduny/Core/Services/MeetingRecorderService.swift (1)
194-288:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix last-chunk duration computation when stitching skips chunks
MeetingChunkStitcher.Result.totalDurationSecondsis computed fromtotalFrameswritten into the stitched output (appendFileincrementstotalFramesas it reads/writes), not fromappendedChunkCount. Also, if the first readable chunk fails mid-append,totalFramesmay still include partially written frames even though that chunk is added toskippedChunks.Because
MeetingRecorderService.stopRecording()sets:
lastDuration = max(0, stitch.totalDurationSeconds - sum(prior manifest.chunks[*].durationSeconds)),
this is only correct if none of the preceding chunks that contribute to the stitched duration were skipped/failed during stitching. Ifstitch.skippedChunkscontains any earlier chunk indices, the subtraction overcounts “prior” duration and can drivelastDurationtoo low (including clamping to0for very short last chunks).🤖 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 `@Diduny/Core/Services/MeetingRecorderService.swift` around lines 194 - 288, stopRecording’s computation of lastDuration wrongly subtracts durations of all preceding chunks from stitch.totalDurationSeconds even when some preceding chunks were skipped during stitching; update the logic in stopRecording() to subtract only the durations of preceding chunks that were actually appended to the stitched output by using MeetingChunkStitcher.Result.skippedChunks (or equivalent) to exclude skipped chunk indices when summing priorDurations from manifest.chunks (i.e., compute priorDurations = sum of manifest.chunks[0..lastIdx-1].durationSeconds where the chunk index is NOT in stitch.skippedChunks), then set lastDuration = max(0, stitch.totalDurationSeconds - priorDurations). Ensure you reference stopRecording(), MeetingChunkStitcher.Result.totalDurationSeconds and .skippedChunks, and manifest.chunks when making the change.
🧹 Nitpick comments (3)
DidunyTests/RecordingModelMigrationTests.swift (1)
129-160: 💤 Low valueConsider using
CaseIterablefor automatic exhaustiveness.The current test uses a manually maintained
allCasesarray and relies on the compiler to enforce switch exhaustiveness. While this works, it could get out of sync if someone adds a new case toProcessingStatusbut forgets to update the array. IfProcessingStatuscan conform toCaseIterable, you could replace lines 133-140 with:let allCases = Recording.ProcessingStatus.allCasesThis would automatically include all cases and eliminate the manual maintenance. However, only apply this if
ProcessingStatusdoesn't have associated values or other constraints that preventCaseIterableconformance.🤖 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 `@DidunyTests/RecordingModelMigrationTests.swift` around lines 129 - 160, Make Recording.ProcessingStatus conform to CaseIterable (if it has no associated values) and update the test_processingStatus_switchIsExhaustive() to use Recording.ProcessingStatus.allCases instead of the manually maintained allCases array; specifically, add CaseIterable to the ProcessingStatus enum declaration and replace the hardcoded array in the test with let allCases = Recording.ProcessingStatus.allCases so the test automatically stays exhaustive.Diduny/App/AppDelegate.swift (1)
39-46: ⚡ Quick winUse
NSLogwith the repo prefixes for the new sleep-flow logs.These new AppDelegate/sleep lifecycle logs should follow the project’s required
NSLogprefix format instead ofLog.app/Log.recording. As per coding guidelines,**/*.swift: Use NSLog() for logging with prefixes: [Diduny] for AppDelegate flow, [Transcription] for cloud API calls, [AppState] for state changes`.Also applies to: 70-76, 319-335
🤖 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 `@Diduny/App/AppDelegate.swift` around lines 39 - 46, Replace the in-file logging calls in AppDelegate's sleep lifecycle with NSLog using the repo prefixes: in the flushActiveRecordingForSleep flow (function flushActiveRecordingForSleep) change Log.recording.info / Log.app uses to NSLog and prefix messages with [Diduny] (e.g. NSLog("[Diduny] ...")). Apply the same replacement for the other identified blocks in this file (around the regions covering lines 70-76 and 319-335) so all AppDelegate/sleep lifecycle messages use NSLog("[Diduny] ...") instead of Log.* calls.Diduny/App/AppDelegate+Hotkeys.swift (1)
96-100: ⚡ Quick winUse
NSLog("[Diduny] …")for these new hotkey-setting logs.These are AppDelegate flow messages, so they should use the repo’s required
NSLogprefix format instead ofLog.app.info(...). As per coding guidelines,**/*.swift: Use NSLog() for logging with prefixes: [Diduny] for AppDelegate flow, [Transcription] for cloud API calls, [AppState] for state changes`.Also applies to: 160-164
🤖 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 `@Diduny/App/AppDelegate`+Hotkeys.swift around lines 96 - 100, Replace the Log.app.info(...) calls in the hotkey handlers with NSLog(...) using the required prefix "[Diduny] " (e.g., in pushToTalkHoldStartDelayChanged(_:) change the Log.app.info message to NSLog("[Diduny] Dictation modifier hold delay changed to: \(String(format: \"%.1f\", delay))s")). Do the same for the other hotkey-setting log statements in AppDelegate+Hotkeys.swift (the other hotkey handler methods around the same section) so all AppDelegate flow messages use NSLog with the "[Diduny]" prefix and preserve the original formatted message content.
🤖 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 `@Diduny/App/AppDelegate.swift`:
- Around line 51-82: The manifest update is being run inside Task { ... } and
returns before persistence completes, so make the InProgressRecordingStore
read/write happen synchronously on the flush path: remove the Task wrapper
around the read/write block that touches recordingId,
manifest.recordingInterruptedBySleep, manifest.lastWriteAt and
manifest.chunks[...] and instead await the store.readManifest(for: recordingId)
and try await store.writeManifest(manifest, for: recordingId) inline (or call
the store's synchronous/blocking write API) so the method performing the flush
(e.g., flushActiveRecordingForSleep()/synchronousFlushForSleep()) does not
return until the manifest has been persisted; if that method cannot be async,
block until completion (DispatchSemaphore or synchronous API) and only call
releaseActivityTokens?() and return flushSucceeded after the write
succeeds/fails and logging completes.
In `@Diduny/Core/Models/Recording.swift`:
- Around line 34-37: The lifecycle bug is that Recording.recoverySource is never
set when a recovered recording is saved: update
RecordingsLibraryStorage.saveRecording (both Data and URL overloads) to accept
and persist an optional RecoverySource and set Recording.recoverySource at
creation for recovered saves; change AppDelegate.recoverRecording(from:) to pass
the recovered RecoverySource into the save path (or construct the recovered
Recording with recoverySource before calling discardRecovery(state:)) so
recovered entries get the "set once at recovery-save time" value, and ensure
RecordingsLibraryStorage.replaceStoredAudioFile continues to preserve
recoverySource when reconstructing recordings.
In `@Diduny/Core/Services/PushToTalkService.swift`:
- Around line 259-280: The isKeyCurrentlyPressed logic uses family-wide flags
(e.g. .shift/.option/.command/.control) for side-specific cases which causes the
right/left pairs to stick when the opposite side is held; update
isKeyCurrentlyPressed so that for side-specific cases (.leftShift, .rightShift,
.leftOption, .rightOption, .leftCommand, .rightCommand, .leftControl,
.rightControl) you determine pressed state by matching the physical keyCode only
(keep the existing numeric keyCode checks like keyCode ==
56/60/58/61/55/54/59/62) and remove the flags.contains(...) check for those
cases, while preserving the capsLock case to still check
flags.contains(.capsLock). This change should be applied inside the
isKeyCurrentlyPressed function and use the existing selectedKey enum case names
to locate the branches.
In `@DidunyTests/SleepFlushCoordinatorTests.swift`:
- Around line 1-2: The test file SleepFlushCoordinatorTests.swift references
AppKit APIs (NSWorkspace used in the tests) but only imports XCTest and
`@testable` import Diduny; add import AppKit at the top of
SleepFlushCoordinatorTests.swift so the NSWorkspace symbols resolve (locate the
file and add the single-line import near the other imports where NSWorkspace is
used in the SleepFlushCoordinatorTests tests).
In `@project.yml`:
- Around line 32-33: The project.yml file was manually edited to set
MARKETING_VERSION and CURRENT_PROJECT_VERSION; revert this manual change so
version fields are managed by CI/git tags instead of hand-editing, removing or
restoring MARKETING_VERSION and CURRENT_PROJECT_VERSION to the
CI-controlled/default state and re-running the generator that populates them;
add (or update) a CI validation step that checks MARKETING_VERSION and
CURRENT_PROJECT_VERSION against the Git tag and fails the build if they diverge,
and add a short inline comment near MARKETING_VERSION to warn contributors not
to edit it by hand.
---
Outside diff comments:
In `@Diduny/Core/Services/MeetingRecorderService.swift`:
- Around line 194-288: stopRecording’s computation of lastDuration wrongly
subtracts durations of all preceding chunks from stitch.totalDurationSeconds
even when some preceding chunks were skipped during stitching; update the logic
in stopRecording() to subtract only the durations of preceding chunks that were
actually appended to the stitched output by using
MeetingChunkStitcher.Result.skippedChunks (or equivalent) to exclude skipped
chunk indices when summing priorDurations from manifest.chunks (i.e., compute
priorDurations = sum of manifest.chunks[0..lastIdx-1].durationSeconds where the
chunk index is NOT in stitch.skippedChunks), then set lastDuration = max(0,
stitch.totalDurationSeconds - priorDurations). Ensure you reference
stopRecording(), MeetingChunkStitcher.Result.totalDurationSeconds and
.skippedChunks, and manifest.chunks when making the change.
---
Nitpick comments:
In `@Diduny/App/AppDelegate.swift`:
- Around line 39-46: Replace the in-file logging calls in AppDelegate's sleep
lifecycle with NSLog using the repo prefixes: in the
flushActiveRecordingForSleep flow (function flushActiveRecordingForSleep) change
Log.recording.info / Log.app uses to NSLog and prefix messages with [Diduny]
(e.g. NSLog("[Diduny] ...")). Apply the same replacement for the other
identified blocks in this file (around the regions covering lines 70-76 and
319-335) so all AppDelegate/sleep lifecycle messages use NSLog("[Diduny] ...")
instead of Log.* calls.
In `@Diduny/App/AppDelegate`+Hotkeys.swift:
- Around line 96-100: Replace the Log.app.info(...) calls in the hotkey handlers
with NSLog(...) using the required prefix "[Diduny] " (e.g., in
pushToTalkHoldStartDelayChanged(_:) change the Log.app.info message to
NSLog("[Diduny] Dictation modifier hold delay changed to: \(String(format:
\"%.1f\", delay))s")). Do the same for the other hotkey-setting log statements
in AppDelegate+Hotkeys.swift (the other hotkey handler methods around the same
section) so all AppDelegate flow messages use NSLog with the "[Diduny]" prefix
and preserve the original formatted message content.
In `@DidunyTests/RecordingModelMigrationTests.swift`:
- Around line 129-160: Make Recording.ProcessingStatus conform to CaseIterable
(if it has no associated values) and update the
test_processingStatus_switchIsExhaustive() to use
Recording.ProcessingStatus.allCases instead of the manually maintained allCases
array; specifically, add CaseIterable to the ProcessingStatus enum declaration
and replace the hardcoded array in the test with let allCases =
Recording.ProcessingStatus.allCases so the test automatically stays exhaustive.
🪄 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
Run ID: c3e02e05-64ba-4a92-a5e1-8078fac44ecb
📒 Files selected for processing (24)
Diduny.xcodeproj/project.pbxprojDiduny/App/AppDelegate+Hotkeys.swiftDiduny/App/AppDelegate+MeetingRecording.swiftDiduny/App/AppDelegate+MeetingTranslationRecording.swiftDiduny/App/AppDelegate.swiftDiduny/Core/Models/Recording.swiftDiduny/Core/Protocols/ServiceProtocols.swiftDiduny/Core/Services/AuthService.swiftDiduny/Core/Services/MeetingChunkStitcher.swiftDiduny/Core/Services/MeetingRecorderService.swiftDiduny/Core/Services/PushToTalkService.swiftDiduny/Core/Services/SleepFlushCoordinator.swiftDiduny/Core/Services/SystemAudioCaptureService.swiftDiduny/Core/Storage/InProgressRecordingManifest.swiftDiduny/Core/Storage/InProgressRecordingStore.swiftDiduny/Core/Storage/RecordingsLibraryStorage.swiftDiduny/Core/Storage/SettingsStorage.swiftDiduny/Features/Settings/ShortcutsSettingsView.swiftDidunyTests/InProgressRecordingStoreTests.swiftDidunyTests/MeetingChunkStitcherTests.swiftDidunyTests/PushToTalkServiceTests.swiftDidunyTests/RecordingModelMigrationTests.swiftDidunyTests/SleepFlushCoordinatorTests.swiftproject.yml
Comment on lines
+34
to
+37
| /// Non-nil when this recording was saved via a recovery path rather than a normal stop. | ||
| /// Drives the "Recovered" badge in the library and the detail-view notice. | ||
| /// Set once at recovery-save time; never cleared. | ||
| var recoverySource: RecoverySource? |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify recoverySource is only set at creation and preserved thereafter
echo "=== Search for assignments to recoverySource ==="
rg -n --type=swift 'recoverySource\s*[:=]' -C 3
echo ""
echo "=== Search for Recording initializer calls with recoverySource ==="
rg -n --type=swift 'Recording\(' -A 15 | rg -A 10 'recoverySource'
echo ""
echo "=== Search for recovery-related save calls ==="
rg -n --type=swift -C 5 'saveRecording|replaceStoredAudioFile'Repository: rmarinsky/Diduny
Length of output: 26538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for RecoverySource usage (excluding tests) ==="
rg -n --type=swift 'RecoverySource' -S . --glob '!DidunyTests/**'
echo ""
echo "=== Search for recoverySource: in non-test Swift files ==="
rg -n --type=swift 'recoverySource\s*:' -S . --glob '!DidunyTests/**'
echo ""
echo "=== Search for any reference to 'Recovered' badge / recovery-save flow ==="
rg -n --type=swift 'Recovered|recovery save|recovery-save|recoverySave|Orphan|orphaned' -S Diduny --glob '!**/*Tests*' --glob '!**/Vendor/**'
echo ""
echo "=== Inspect RecordingsLibraryStorage.swift around replaceStoredAudioFile and related metadata write ==="
rg -n --type=swift 'replaceStoredAudioFile|saveMetadata|Recording\(' Diduny/Core/Storage/RecordingsLibraryStorage.swift
echo ""
sed -n '240,360p' Diduny/Core/Storage/RecordingsLibraryStorage.swiftRepository: rmarinsky/Diduny
Length of output: 6741
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for orphanedSession / partiallyRecovered usages (non-tests) ==="
rg -n --type=swift 'orphanedSession|partiallyRecovered' Diduny --glob '!**/*Tests*'
echo ""
echo "=== Inspect AppDelegate recovery/orphaned flow around checkForOrphanedRecordings ==="
sed -n '240,520p' Diduny/App/AppDelegate.swift | sed -n '1,300p'
echo ""
echo "=== Inspect RecordingsLibraryStorage.swift saveRecording initializer calls (look for recoverySource argument) ==="
sed -n '1,220p' Diduny/Core/Storage/RecordingsLibraryStorage.swift
echo ""
echo "=== Inspect recovery detector/manager code that assembles recovered recordings (if present) ==="
rg -n --type=swift 'OrphanedRecordingDetector|Recovery|recoverySave|recovery save|assemble' Diduny/Core/Storage Diduny/App --glob '!**/*Tests*'Repository: rmarinsky/Diduny
Length of output: 23898
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Inspect Recording.swift around RecoverySource + recoverySource property and Codable/init ==="
rg -n --type=swift 'enum RecoverySource|var recoverySource|CodingKeys|init\(|encode|decode' Diduny/Core/Models/Recording.swift
sed -n '1,220p' Diduny/Core/Models/Recording.swift
echo ""
echo "=== Inspect RecordingsLibraryStorage.swift loadMetadata/saveMetadata (look for recoverySource encoding/decoding) ==="
rg -n --type=swift 'loadMetadata|saveMetadata|JSONEncoder|JSONDecoder|recoverySource' Diduny/Core/Storage/RecordingsLibraryStorage.swift
sed -n '220,340p' Diduny/Core/Storage/RecordingsLibraryStorage.swiftRepository: rmarinsky/Diduny
Length of output: 8437
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for OrphanedRecordingDetector implementation/usages ==="
rg -n --type=swift 'OrphanedRecordingDetector|orphaned recording|orphanedRecording|orphanedSession|RecoverySource' Diduny --glob '!**/*Tests*'
echo ""
echo "=== Search for 'saveMetadata()' / direct 'recordings[' mutation with recoverySource preservation ==="
rg -n --type=swift 'recordings\[\s*\w+\s*\]\s*=\s*Recording\(|Recording\(' Diduny/Core/Storage --glob '!**/*Tests*'Repository: rmarinsky/Diduny
Length of output: 1784
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search: where is tempFilePath used? (non-tests) ==="
rg -n --type=swift 'tempFilePath' Diduny --glob '!**/*Tests*'
echo ""
echo "=== Search: any RecordingsLibraryStorage.saveRecording calls within recovery flow (AppDelegate.swift) ==="
rg -n --type=swift 'recoverRecording\(from:|discardRecovery\(state:|checkForOrphanedRecordings|hasOrphanedRecording' Diduny/App/AppDelegate.swift
echo ""
rg -n --type=swift 'RecordingsLibraryStorage\.shared\.saveRecording\(' Diduny/App/AppDelegate.swift --glob '!**/*Tests*'
echo ""
rg -n --type=swift 'saveRecording\(' Diduny/App/AppDelegate.swift --glob '!**/*Tests*'
echo ""
echo "=== Search: any other files create Recording with recoverySource: (non-tests) ==="
rg -n --type=swift 'recoverySource\s*:' Diduny --glob '!**/*Tests*'
echo ""
rg -n --type=swift 'recoverySource\s*=' Diduny --glob '!**/*Tests*'
echo ""
rg -n --type=swift 'RecoveryStateManager\.shared\.loadState\(' Diduny --glob '!**/*Tests*'Repository: rmarinsky/Diduny
Length of output: 1388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for 'orphaned' across non-test code ==="
rg -n --type=swift 'orphaned|Orphaned' Diduny --glob '!**/*Tests*'
echo ""
echo "=== Search for 'recoverySource' across non-test code ==="
rg -n --type=swift 'recoverySource' Diduny --glob '!**/*Tests*'
echo ""
echo "=== List files under Diduny/Core/Storage (to find missing detector/assembler) ==="
ls -R Diduny/Core/Storage | sed -n '1,200p'
echo ""
echo "=== Inspect RecoveryStateManager.swift ==="
sed -n '1,200p' Diduny/Core/Storage/RecoveryStateManager.swiftRepository: rmarinsky/Diduny
Length of output: 4321
Fix recoverySource lifecycle contract: recovery flow never sets it in production code, but replacement preserves it.
RecordingsLibraryStorage.saveRecording(...)(both Data + URL overloads) createsRecordingwithoutrecoverySource, so newly saved recovered entries default tonil(no “set once at recovery-save time” path found).AppDelegate.recoverRecording(from:)only transcribes/cleans text and then callsdiscardRecovery(state:); it doesn’t create a recoveredRecordingin the library.RecordingsLibraryStorage.replaceStoredAudioFile(...)is the only place that reconstructsRecordingwithrecoverySource: recording.recoverySource, and no other non-test code mutates/clearsrecoverySource.
🤖 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 `@Diduny/Core/Models/Recording.swift` around lines 34 - 37, The lifecycle bug
is that Recording.recoverySource is never set when a recovered recording is
saved: update RecordingsLibraryStorage.saveRecording (both Data and URL
overloads) to accept and persist an optional RecoverySource and set
Recording.recoverySource at creation for recovered saves; change
AppDelegate.recoverRecording(from:) to pass the recovered RecoverySource into
the save path (or construct the recovered Recording with recoverySource before
calling discardRecovery(state:)) so recovered entries get the "set once at
recovery-save time" value, and ensure
RecordingsLibraryStorage.replaceStoredAudioFile continues to preserve
recoverySource when reconstructing recordings.
Comment on lines
+32
to
+33
| MARKETING_VERSION: "1.14.4" | ||
| CURRENT_PROJECT_VERSION: "4" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify CI versioning approach and check for conflicting tags
echo "=== CI configuration files ==="
fd -t f 'yml|yaml' .github/workflows | head -5
echo ""
echo "=== Recent version tags ==="
git tag --sort=-version:refname | grep '^v' | head -10
echo ""
echo "=== Current branch version settings ==="
grep -A2 'MARKETING_VERSION\|CURRENT_PROJECT_VERSION' project.ymlRepository: rmarinsky/Diduny
Length of output: 402
Disallow hand-editing MARKETING_VERSION in project.yml (workflow violation)
project.yml now contains:
MARKETING_VERSION: "1.14.4"
CURRENT_PROJECT_VERSION: "4"This conflicts with the documented workflow that CI derives versioning from git tags and that MARKETING_VERSION in project.yml should not be manually edited—risking mismatches between the tag/release metadata and what’s embedded in the app/update artifacts.
🤖 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 `@project.yml` around lines 32 - 33, The project.yml file was manually edited
to set MARKETING_VERSION and CURRENT_PROJECT_VERSION; revert this manual change
so version fields are managed by CI/git tags instead of hand-editing, removing
or restoring MARKETING_VERSION and CURRENT_PROJECT_VERSION to the
CI-controlled/default state and re-running the generator that populates them;
add (or update) a CI validation step that checks MARKETING_VERSION and
CURRENT_PROJECT_VERSION against the Git tag and fails the build if they diverge,
and add a short inline comment near MARKETING_VERSION to warn contributors not
to edit it by hand.
…t check The proxy now refuses realtime WS upgrades with HTTP 402 once a user is over the usage quota. Previously a 402 (when the local usage pre-check was nil or stale) surfaced as a failed receive → generic reconnect storm (2s/4s/8s) and a "Connection lost" message instead of the real reason. - Detect a 402 upgrade response in handleDisconnect (synchronously, to stop the reconnect) and on the initial config send, mapping it to a typed usageLimitExceeded error + a usage refresh. - shouldAcceptRealtimeTranscript: measure trimmed length for the 120-char substantiality threshold so a whitespace-padded transcript can't masquerade as substantial. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- AppDelegate (sleep flush): persist the in-progress manifest synchronously
before flushActiveRecordingForSleep() returns. The write was deferred to a
Task{} and could be lost if the app suspended first, leaving recovery with
stale state. Block on a detached task (no MainActor deadlock) with a 2s guard.
- PushToTalkService: distinguish left/right modifiers via device-dependent
masks (NX_DEVICE*KEYMASK) instead of family flags (.shift/.option/...), so a
side-specific key detects its own key-up while the opposite side is held —
previously hold-to-record could stick.
- SleepFlushCoordinatorTests: import AppKit (uses NSWorkspace) so the test
target compiles under CI.
- project.yml: reset MARKETING_VERSION/CURRENT_PROJECT_VERSION to neutral
placeholders — CI derives the real version from the git tag (per CLAUDE.md);
hand-editing risked tag/artifact mismatch.
- Recording.recoverySource: correct the doc comment to reality (no save path
sets it yet) + TODO, instead of claiming an unimplemented contract.
Build + full DidunyTests suite pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
Diduny/Core/Models/Recording.swift (1)
43-43: ⚡ Quick winEnforce the “never cleared” contract in the model.
Line 43 is mutable (
var), but the contract above says the value should be preserved once set. Making this immutable prevents accidental clearing and aligns with the persistence invariant.Suggested change
- var recoverySource: RecoverySource? + let recoverySource: RecoverySource?🤖 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 `@Diduny/Core/Models/Recording.swift` at line 43, Change the mutable property "recoverySource" to an immutable constant to enforce the "never cleared" contract: replace "var recoverySource: RecoverySource?" with "let recoverySource: RecoverySource?" in the Recording model, then update any initializers, Codable init(from:)/Decodable paths, factory methods, and places that previously assigned to recoverySource so they set it at construction time instead of later; if any code mutates recoverySource, refactor that logic to produce a new Recording or accept the value in the initializer.Diduny/Core/Services/CloudRealtimeService.swift (1)
462-462: ⚡ Quick winUse the required transcription log format here.
This new cloud-API log should use
NSLog("[Transcription] ...")so it follows the repo’s searchable logging contract instead of going throughLog.transcription.As per coding guidelines,
**/*.swift: Use NSLog() for logging with prefixes: [Diduny] for AppDelegate flow, [Transcription] for cloud API calls, [AppState] for state changes.🤖 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 `@Diduny/Core/Services/CloudRealtimeService.swift` at line 462, Replace the Log.transcription.warning call with an NSLog-based transcription log so it follows the repo logging contract: locate the call to Log.transcription.warning("Cloud RT: WS upgrade returned 402 — usage limit, not reconnecting") in CloudRealtimeService (the WS upgrade/response handling method) and change it to emit an NSLog with the [Transcription] prefix and the same message (i.e., use NSLog("[Transcription] Cloud RT: WS upgrade returned 402 — usage limit, not reconnecting")). Ensure no other Log.transcription usage remains for this cloud-API message.Diduny/App/AppDelegate.swift (1)
87-90: ⚡ Quick winUse
NSLogfor the new AppDelegate sleep-path log.This adds AppDelegate-flow logging through
Log.recording.error, which diverges from the repo logging contract for Swift files. Please switch this timeout log toNSLog("[Diduny] ...")so lifecycle diagnostics stay consistent. As per coding guidelines, "**/*.swift: Use NSLog() for logging with prefixes: [Diduny] for AppDelegate flow, [Transcription] for cloud API calls, [AppState] for state changes".🤖 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 `@Diduny/App/AppDelegate.swift` around lines 87 - 90, The timeout log in the AppDelegate sleep path currently uses Log.recording.error (see sem.wait timeout branch and the existing log string) which violates the Swift logging contract; replace that Log.recording.error call with an NSLog call using the AppDelegate prefix, e.g. NSLog("[Diduny] manifest update timed out (2s) — proceeding without confirmed persist"), preserving the original message text and context around the sem.wait timeout handling so lifecycle diagnostics use NSLog as required.
🤖 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 `@Diduny/App/AppDelegate.swift`:
- Around line 53-61: The sleep-flush path is blocking for up to 2s via the
DispatchSemaphore wait around a Task.detached; change this so persistence is
bounded to the coordinator budget: either perform the manifest write
synchronously on the current thread (remove Task.detached + semaphore) or, if
keeping the detached task, cap the semaphore wait to the SleepFlushCoordinator
budget (~250ms) and treat a timeout as a flush failure (log/report/return
failure) rather than proceeding as if the write succeeded; make the same change
for the other instance at the 87-90 region, ensure sem.signal() runs on all code
paths, and update any callers of SleepFlushCoordinator.flushCurrentChunk to
handle a failed/timed-out flush result.
In `@Diduny/Core/Services/CloudRealtimeService.swift`:
- Around line 433-442: The code reads UsageService.shared.cachedUsage into
`usage` before calling await UsageService.shared.refresh(), so the created
RealtimeTranscriptionError uses a stale snapshot; change the order in
usageLimitUpgradeError() to call await UsageService.shared.refresh() first, then
read UsageService.shared.cachedUsage (falling back to the original snapshot only
if the refreshed cache is nil), and apply the same change in the connect path
that constructs the usageLimitExceeded error (the connect method) so the UI gets
the refreshed quota values rather than the pre-refresh snapshot.
- Around line 456-474: The 402 branch exits early without tearing down the live
socket, leaving webSocketTask/urlSession retained and pingTask running; before
returning, cancel and cleanup the socket state (cancel webSocketTask, invalidate
or finish the urlSession if owned, clear/await/ cancel pingTask) and reset any
connection-related properties so the normal cleanup paths aren't required;
perform this teardown inside the same Task block (or synchronously before it)
that calls UsageService and before calling self.onError? /
self.onConnectionStatusChanged? so that webSocketTask, urlSession and pingTask
are stopped (refer to webSocketTask, urlSession, pingTask, disconnect()).
---
Nitpick comments:
In `@Diduny/App/AppDelegate.swift`:
- Around line 87-90: The timeout log in the AppDelegate sleep path currently
uses Log.recording.error (see sem.wait timeout branch and the existing log
string) which violates the Swift logging contract; replace that
Log.recording.error call with an NSLog call using the AppDelegate prefix, e.g.
NSLog("[Diduny] manifest update timed out (2s) — proceeding without confirmed
persist"), preserving the original message text and context around the sem.wait
timeout handling so lifecycle diagnostics use NSLog as required.
In `@Diduny/Core/Models/Recording.swift`:
- Line 43: Change the mutable property "recoverySource" to an immutable constant
to enforce the "never cleared" contract: replace "var recoverySource:
RecoverySource?" with "let recoverySource: RecoverySource?" in the Recording
model, then update any initializers, Codable init(from:)/Decodable paths,
factory methods, and places that previously assigned to recoverySource so they
set it at construction time instead of later; if any code mutates
recoverySource, refactor that logic to produce a new Recording or accept the
value in the initializer.
In `@Diduny/Core/Services/CloudRealtimeService.swift`:
- Line 462: Replace the Log.transcription.warning call with an NSLog-based
transcription log so it follows the repo logging contract: locate the call to
Log.transcription.warning("Cloud RT: WS upgrade returned 402 — usage limit, not
reconnecting") in CloudRealtimeService (the WS upgrade/response handling method)
and change it to emit an NSLog with the [Transcription] prefix and the same
message (i.e., use NSLog("[Transcription] Cloud RT: WS upgrade returned 402 —
usage limit, not reconnecting")). Ensure no other Log.transcription usage
remains for this cloud-API message.
🪄 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
Run ID: 85034382-3f7d-471d-b119-22941de14c7f
📒 Files selected for processing (7)
Diduny/App/AppDelegate+MeetingRecording.swiftDiduny/App/AppDelegate.swiftDiduny/Core/Models/Recording.swiftDiduny/Core/Services/CloudRealtimeService.swiftDiduny/Core/Services/PushToTalkService.swiftDidunyTests/SleepFlushCoordinatorTests.swiftproject.yml
🚧 Files skipped from review as they are similar to previous changes (3)
- DidunyTests/SleepFlushCoordinatorTests.swift
- Diduny/Core/Services/PushToTalkService.swift
- Diduny/App/AppDelegate+MeetingRecording.swift
Comment on lines
+53
to
+61
| // Persist the manifest synchronously before returning: the app can be | ||
| // suspended the instant this sleep-flush returns, so a deferred async | ||
| // write could be lost and leave recovery reading stale state after | ||
| // wake/crash. Block on a detached task (detached → not MainActor-bound, | ||
| // so waiting on the main thread can't deadlock the actor) with a short | ||
| // timeout so a wedged store can't hang the sleep transition. | ||
| let sem = DispatchSemaphore(value: 0) | ||
| Task.detached(priority: .userInitiated) { | ||
| defer { sem.signal() } |
There was a problem hiding this comment.
Keep the sleep-flush path within the coordinator’s timing budget.
SleepFlushCoordinator.flushCurrentChunk is documented to finish on the power-management thread within ~250 ms, but this path can now block for up to 2 seconds on actor/file I/O. That can stall system sleep and still leaves you without a confirmed manifest write if the timeout fires. Please move this persistence onto a truly bounded synchronous path, or at minimum cap the wait to the coordinator budget and treat timeout as a failed flush.
Also applies to: 87-90
🤖 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 `@Diduny/App/AppDelegate.swift` around lines 53 - 61, The sleep-flush path is
blocking for up to 2s via the DispatchSemaphore wait around a Task.detached;
change this so persistence is bounded to the coordinator budget: either perform
the manifest write synchronously on the current thread (remove Task.detached +
semaphore) or, if keeping the detached task, cap the semaphore wait to the
SleepFlushCoordinator budget (~250ms) and treat a timeout as a flush failure
(log/report/return failure) rather than proceeding as if the write succeeded;
make the same change for the other instance at the 87-90 region, ensure
sem.signal() runs on all code paths, and update any callers of
SleepFlushCoordinator.flushCurrentChunk to handle a failed/timed-out flush
result.
Comment on lines
+433
to
+442
| private func usageLimitUpgradeError() async -> RealtimeTranscriptionError? { | ||
| guard (webSocketTask?.response as? HTTPURLResponse)?.statusCode == 402 else { | ||
| return nil | ||
| } | ||
| let usage = await UsageService.shared.cachedUsage | ||
| await UsageService.shared.refresh() | ||
| return .usageLimitExceeded( | ||
| usedHours: usage?.usedHours ?? 0, | ||
| limitHours: usage?.limitHours ?? 5 | ||
| ) |
There was a problem hiding this comment.
Build the usage-limit error from the refreshed cache.
Both 402 paths snapshot cachedUsage before refresh(), then format usageLimitExceeded from that stale snapshot. The connect path bubbles error.localizedDescription straight into UI state, so a real quota hit can still show fallback numbers like 0.0h / 5h even when the refresh succeeded. Refresh first, then read the updated cache, and only fall back to the pre-refresh snapshot if the refresh did not populate it.
Also applies to: 465-470
🤖 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 `@Diduny/Core/Services/CloudRealtimeService.swift` around lines 433 - 442, The
code reads UsageService.shared.cachedUsage into `usage` before calling await
UsageService.shared.refresh(), so the created RealtimeTranscriptionError uses a
stale snapshot; change the order in usageLimitUpgradeError() to call await
UsageService.shared.refresh() first, then read UsageService.shared.cachedUsage
(falling back to the original snapshot only if the refreshed cache is nil), and
apply the same change in the connect path that constructs the usageLimitExceeded
error (the connect method) so the UI gets the refreshed quota values rather than
the pre-refresh snapshot.
Comment on lines
+456
to
+474
| // A refused WS upgrade (HTTP 402 usage limit) lands here via the receive | ||
| // loop with no close code. Reconnecting is futile — the server will keep | ||
| // refusing — and would surface a generic "Connection lost" instead of the | ||
| // real reason. Detect it synchronously to stop the reconnect, then surface | ||
| // the typed usage error with the best numbers we have. | ||
| if (webSocketTask?.response as? HTTPURLResponse)?.statusCode == 402 { | ||
| Log.transcription.warning("Cloud RT: WS upgrade returned 402 — usage limit, not reconnecting") | ||
| Task { [weak self] in | ||
| guard let self else { return } | ||
| let usage = await UsageService.shared.cachedUsage | ||
| await UsageService.shared.refresh() | ||
| self.onError?(RealtimeTranscriptionError.usageLimitExceeded( | ||
| usedHours: usage?.usedHours ?? 0, | ||
| limitHours: usage?.limitHours ?? 5 | ||
| )) | ||
| self.onConnectionStatusChanged?(.failed("Cloud usage limit reached")) | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
Tear down the failed socket before returning on 402.
This branch returns before either cleanup path runs. Since the ping loop is already started earlier, the dead webSocketTask/urlSession stay retained and pingTask keeps firing until someone manually calls disconnect(). Cancel/reset the live socket state here before returning.
Suggested fix
if (webSocketTask?.response as? HTTPURLResponse)?.statusCode == 402 {
+ pingTask?.cancel()
+ pingTask = nil
+ receiveTask?.cancel()
+ receiveTask = nil
+ webSocketTask?.cancel(with: .normalClosure, reason: nil)
+ webSocketTask = nil
+ urlSession?.invalidateAndCancel()
+ urlSession = nil
Log.transcription.warning("Cloud RT: WS upgrade returned 402 — usage limit, not reconnecting")
Task { [weak self] in
guard let self else { return }
let usage = await UsageService.shared.cachedUsage
await UsageService.shared.refresh()📝 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
| // A refused WS upgrade (HTTP 402 usage limit) lands here via the receive | |
| // loop with no close code. Reconnecting is futile — the server will keep | |
| // refusing — and would surface a generic "Connection lost" instead of the | |
| // real reason. Detect it synchronously to stop the reconnect, then surface | |
| // the typed usage error with the best numbers we have. | |
| if (webSocketTask?.response as? HTTPURLResponse)?.statusCode == 402 { | |
| Log.transcription.warning("Cloud RT: WS upgrade returned 402 — usage limit, not reconnecting") | |
| Task { [weak self] in | |
| guard let self else { return } | |
| let usage = await UsageService.shared.cachedUsage | |
| await UsageService.shared.refresh() | |
| self.onError?(RealtimeTranscriptionError.usageLimitExceeded( | |
| usedHours: usage?.usedHours ?? 0, | |
| limitHours: usage?.limitHours ?? 5 | |
| )) | |
| self.onConnectionStatusChanged?(.failed("Cloud usage limit reached")) | |
| } | |
| return | |
| } | |
| // A refused WS upgrade (HTTP 402 usage limit) lands here via the receive | |
| // loop with no close code. Reconnecting is futile — the server will keep | |
| // refusing — and would surface a generic "Connection lost" instead of the | |
| // real reason. Detect it synchronously to stop the reconnect, then surface | |
| // the typed usage error with the best numbers we have. | |
| if (webSocketTask?.response as? HTTPURLResponse)?.statusCode == 402 { | |
| pingTask?.cancel() | |
| pingTask = nil | |
| receiveTask?.cancel() | |
| receiveTask = nil | |
| webSocketTask?.cancel(with: .normalClosure, reason: nil) | |
| webSocketTask = nil | |
| urlSession?.invalidateAndCancel() | |
| urlSession = nil | |
| Log.transcription.warning("Cloud RT: WS upgrade returned 402 — usage limit, not reconnecting") | |
| Task { [weak self] in | |
| guard let self else { return } | |
| let usage = await UsageService.shared.cachedUsage | |
| await UsageService.shared.refresh() | |
| self.onError?(RealtimeTranscriptionError.usageLimitExceeded( | |
| usedHours: usage?.usedHours ?? 0, | |
| limitHours: usage?.limitHours ?? 5 | |
| )) | |
| self.onConnectionStatusChanged?(.failed("Cloud usage limit reached")) | |
| } | |
| return | |
| } |
🤖 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 `@Diduny/Core/Services/CloudRealtimeService.swift` around lines 456 - 474, The
402 branch exits early without tearing down the live socket, leaving
webSocketTask/urlSession retained and pingTask running; before returning, cancel
and cleanup the socket state (cancel webSocketTask, invalidate or finish the
urlSession if owned, clear/await/ cancel pingTask) and reset any
connection-related properties so the normal cleanup paths aren't required;
perform this teardown inside the same Task block (or synchronously before it)
that calls UsageService and before calling self.onError? /
self.onConnectionStatusChanged? so that webSocketTask, urlSession and pingTask
are stopped (refer to webSocketTask, urlSession, pingTask, disconnect()).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Diduny/Core/Services/AsyncTranscriptionJobService.swift (1)
185-200:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStatus-poll failures currently abort retry loop prematurely
When SSE fails, a transient failure in
getJobStatus(jobId:)(Line 185) throws out oftranscribeWithRetryimmediately, bypassing the deadline-based retry behavior. That turns a recoverable network blip into a hard failure.Suggested fix
} catch { sseFailures += 1 Log.transcription.warning("SSE stream failed (attempt \(sseFailures)): \(error)") // Check if job finished while disconnected - let status = try await getJobStatus(jobId: submission.jobId) - if status.status == "completed", let result = status.result { - return result.text - } - if status.status == "error" { - throw TranscriptionError.apiError(status.error ?? "Transcription failed") - } - if let parsed = JobStatus(rawValue: status.status) { - onUpdate(parsed) + do { + let status = try await getJobStatus(jobId: submission.jobId) + if status.status == "completed", let result = status.result { + return result.text + } + if status.status == "error" { + throw TranscriptionError.apiError(status.error ?? "Transcription failed") + } + if let parsed = JobStatus(rawValue: status.status) { + onUpdate(parsed) + } + } catch is CancellationError { + throw CancellationError() + } catch { + Log.transcription.warning("Status poll failed after SSE disconnect: \(error)") } // Still in progress. SSE is best-effort; keep polling/retrying until // the server-side job reaches a terminal state or the long job timeout. try Task.checkCancellation() let delaySeconds = min(Double(max(sseFailures, 1)) * 2, 30) try await Task.sleep(nanoseconds: UInt64(delaySeconds * 1_000_000_000)) }🤖 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 `@Diduny/Core/Services/AsyncTranscriptionJobService.swift` around lines 185 - 200, The poll call to getJobStatus(jobId:) inside transcribeWithRetry currently lets any thrown error escape and abort retries; wrap the call in a do-catch inside transcribeWithRetry so transient errors increment the sseFailures counter, log or capture the error, and fall through to the existing backoff sleep/loop (rather than rethrowing) until the longJobTimeout/deadline or Task cancellation is reached; only rethrow or throw a TranscriptionError.apiError when the deadline has passed or the error is non-transient, and keep the existing Task.checkCancellation, delay calculation, and onUpdate(JobStatus(rawValue:)) handling.
🤖 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 `@Diduny/Core/Services/AsyncTranscriptionJobService.swift`:
- Line 182: Replace the Log.transcription.warning call in
AsyncTranscriptionJobService (the SSE stream failure handling) with an NSLog
call that uses the "[Transcription]" prefix and includes the error details;
specifically, change the logging expression that currently reads
Log.transcription.warning("SSE stream failed (attempt \(sseFailures)):
\(error)") to an NSLog invocation that starts with "[Transcription]" and formats
the same message including sseFailures and the error for consistent repo logging
conventions.
---
Outside diff comments:
In `@Diduny/Core/Services/AsyncTranscriptionJobService.swift`:
- Around line 185-200: The poll call to getJobStatus(jobId:) inside
transcribeWithRetry currently lets any thrown error escape and abort retries;
wrap the call in a do-catch inside transcribeWithRetry so transient errors
increment the sseFailures counter, log or capture the error, and fall through to
the existing backoff sleep/loop (rather than rethrowing) until the
longJobTimeout/deadline or Task cancellation is reached; only rethrow or throw a
TranscriptionError.apiError when the deadline has passed or the error is
non-transient, and keep the existing Task.checkCancellation, delay calculation,
and onUpdate(JobStatus(rawValue:)) handling.
🪄 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
Run ID: 39b9eb56-67c4-449b-8f1c-fb4e163ac90a
📒 Files selected for processing (1)
Diduny/Core/Services/AsyncTranscriptionJobService.swift
| retries += 1 | ||
| Log.transcription.warning("SSE stream failed (attempt \(retries)/\(self.maxRetries)): \(error)") | ||
| sseFailures += 1 | ||
| Log.transcription.warning("SSE stream failed (attempt \(sseFailures)): \(error)") |
There was a problem hiding this comment.
Use NSLog with [Transcription] prefix for this cloud API log
This changed line uses Log.transcription... instead of the required Swift logging convention from the repo guideline.
As per coding guidelines, "**/*.swift: Use NSLog() for logging with prefixes: [Diduny] for AppDelegate flow, [Transcription] for cloud API calls, [AppState] for state changes".
🤖 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 `@Diduny/Core/Services/AsyncTranscriptionJobService.swift` at line 182, Replace
the Log.transcription.warning call in AsyncTranscriptionJobService (the SSE
stream failure handling) with an NSLog call that uses the "[Transcription]"
prefix and includes the error details; specifically, change the logging
expression that currently reads Log.transcription.warning("SSE stream failed
(attempt \(sseFailures)): \(error)") to an NSLog invocation that starts with
"[Transcription]" and formats the same message including sseFailures and the
error for consistent repo logging conventions.
Source: Coding guidelines
…nical-proxy # Conflicts: # Diduny/App/AppDelegate.swift # Diduny/Core/Models/Recording.swift # Diduny/Core/Services/PushToTalkService.swift # DidunyTests/SleepFlushCoordinatorTests.swift # project.yml
rmarinsky
changed the title
Contributor
|
🔖 On merge this PR will release v1.14.5 ( |
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
This PR hardens the meeting transcription path after the Fly.io incident where the jobs service completed transcription, but the macOS app had already stopped waiting after SSE reconnect failures.
Changes
result.text.mainafter PR [codex] prepare 1.14.4 canonical proxy release #38 so this PR is mergeable and does not revert release version1.14.4.Verification
xcodebuild -project Diduny.xcodeproj -scheme "Diduny DEV" -configuration Debug build-> BUILD SUCCEEDED.PushToTalkService; fixed and reran successfully.Release note
No direct Fly.io deploy from local. Release should continue through PR review and GitHub Actions.