fix: harden microphone and recording reliability#235
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR tracks legacy microphone IDs and reconciles selected devices on refresh; hardens recording lifecycle (start/stop/pause/mute/cleanup) and media resumption; propagates transcription duration and improves cancellation; converts playback completion to an async stream correlated by playbackID; and includes UI/menu tweaks, schema migration, and new tests. ChangesMicrophone Selection Stability and Recording Robustness
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 @.changeset/83606f78.md:
- Line 5: Update the changeset description in .changeset/83606f78.md to include
the GitHub issue or PR reference in the required format by appending "
(#<issue-number>)" to the summary line (e.g., change "Refresh microphones
reliably, harden recording cleanup, and polish the settings overlay behavior."
to "Refresh microphones reliably, harden recording cleanup, and polish the
settings overlay behavior (`#123`)"); ensure the user-facing impact remains and
the PR/issue number is accurate and present in that single-line summary.
In `@Hex/App/HexAppDelegate.swift`:
- Around line 126-127: The calls to settingsWindow.center() and
settingsWindow.setFrameAutosaveName("Settings") are in the wrong order so the
autosaved frame gets overridden by centering; swap them so you call
settingsWindow.center() first and then
settingsWindow.setFrameAutosaveName("Settings") (i.e., move the
setFrameAutosaveName call after the center call) to let AppKit restore the saved
frame when available.
In `@Hex/Clients/RecordingClient.swift`:
- Around line 1453-1458: In RecordingClientLive.cleanup(),
stopObservingSystemChanges() must be called before any await to prevent queued
environment-change handlers from restarting the capture path during teardown;
reorder the calls so that stopObservingSystemChanges() is invoked immediately
after endRecordingSession() and before await resumeMediaIfNeeded(), while
preserving subsequent calls to stopCaptureController(reason: "cleanup") and
releaseRecorder(reason: "cleanup") (also ensure any related listeners are
deregistered in stopObservingSystemChanges()).
In `@Hex/Features/Settings/SettingsFeature.swift`:
- Around line 537-541: The migration fails when Core Audio renumbers devices
because legacyID is generated from the current AudioDeviceID in RecordingClient,
so stored legacy numeric IDs can no longer be resolved in the
availableInputDevicesLoaded handler; fix by persisting a mapping of previous
numeric IDs to stable UIDs across refreshes in RecordingClient (or running a
startup migration before device enumeration) and use that mapping when handling
case .availableInputDevicesLoaded to translate the stored selectedMicrophoneID
(legacy numeric ID) into the device.id (stable UID) before assigning it via
state.$hexSettings.withLock; update RecordingClient (where legacyID is created)
to carry forward previous numeric IDs keyed by UID and expose a lookup method
(or perform an early migration in SettingsFeature/resolvePreferredInputDevice)
so the lookup uses past numeric→UID mappings rather than only the current
AudioDeviceID.
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Line 401: Replace the direct transcriptionFeatureLogger.notice call with the
unified HexLog helper (use category .transcription or add it) and mark the
captured filename as private; specifically, when logging the file name from
capturedURL.lastPathComponent pass it with a privacy annotation (privacy:
.private) and keep the transcript/result text private as well if included.
Locate the existing transcriptionFeatureLogger.notice(...) invocation and swap
it to use HexLog(.transcription).notice(...) (or equivalent) and annotate
capturedURL.lastPathComponent with privacy: .private.
- Around line 404-406: In the catch block after calling
transcription.transcribe(...), detect cancellation and treat it as a no-op: if
the thrown error is a CancellationError (or Task.isCancelled), do not call await
send(.transcriptionError(...)) or update state; otherwise log and send
.transcriptionError as before. Update the catch to check for CancellationError
(or Task.isCancelled) and only dispatch .transcriptionError for non-cancellation
errors, referencing the existing transcription.transcribe(...),
.transcriptionError, and CancelID.transcription flow.
- Around line 386-387: The code incorrectly treats any stop result as real audio
because state.isRecording flips true before recordingStart finishes; update the
flow to distinguish "capture actually started" from just the state flag: either
add a new boolean (e.g., captureStarted) set only after
recording.startRecording() completes in the recordingStart effect and check that
before calling recording.stopRecording()/using its URL, or change
recording.stopRecording() to return an explicit enum/result (e.g.,
.captured(URL) | .ignored) and only assign/transcribe when the result is
.captured; reference recording.startRecording(), recording.stopRecording(), and
state.isRecording to implement the chosen approach so stale/ignored-stop URLs
are discarded.
🪄 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: 30f8759a-f46c-48e4-991a-1848f85d6fee
📒 Files selected for processing (15)
.changeset/83606f78.mdHex.xcodeproj/project.pbxprojHex/App/HexApp.swiftHex/App/HexAppDelegate.swiftHex/App/MenuBarCopyLastTranscriptButton.swiftHex/Clients/RecordingClient.swiftHex/Features/App/AppFeature.swiftHex/Features/History/HistoryFeature.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Transcription/TranscriptionFeature.swiftHex/Views/InvisibleWindow.swiftHexCore/Sources/HexCore/Settings/HexSettings.swiftHexCore/Tests/HexCoreTests/HexSettingsMigrationTests.swiftHexTests/HistoryPlaybackTests.swiftHexTests/RecordingRaceTests.swift
| "hex-app": patch | ||
| --- | ||
|
|
||
| Refresh microphones reliably, harden recording cleanup, and polish the settings overlay behavior. |
Contributor
There was a problem hiding this comment.
Add GitHub issue reference to the changeset description.
As per coding guidelines, changeset descriptions should include the GitHub issue or PR number in the format 'Summary text (#123)'.
📝 Proposed fix
-Refresh microphones reliably, harden recording cleanup, and polish the settings overlay behavior.
+Refresh microphones reliably, harden recording cleanup, and polish the settings overlay behavior (`#234`).As per coding guidelines for .changeset/*.md: "Include user-facing impact and GitHub issue/PR number in the format 'Improve Fn hotkey stability (#89)'."
📝 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
| Refresh microphones reliably, harden recording cleanup, and polish the settings overlay behavior. | |
| Refresh microphones reliably, harden recording cleanup, and polish the settings overlay behavior (`#234`). |
🤖 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 @.changeset/83606f78.md at line 5, Update the changeset description in
.changeset/83606f78.md to include the GitHub issue or PR reference in the
required format by appending " (#<issue-number>)" to the summary line (e.g.,
change "Refresh microphones reliably, harden recording cleanup, and polish the
settings overlay behavior." to "Refresh microphones reliably, harden recording
cleanup, and polish the settings overlay behavior (`#123`)"); ensure the
user-facing impact remains and the PR/issue number is accurate and present in
that single-line summary.
Comment on lines
+126
to
127
| settingsWindow.setFrameAutosaveName("Settings") | ||
| settingsWindow.center() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant section with line numbers
FILE="Hex/App/HexAppDelegate.swift"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
# Show around the referenced lines (120-140)
sed -n '110,150p' "$FILE" | cat -n
# Also search for setFrameAutosaveName / center usage to see if center is conditional elsewhere
echo "---- occurrences of setFrameAutosaveName ----"
rg -n "setFrameAutosaveName\\(" "$FILE" || true
echo "---- occurrences of center() on settingsWindow ----"
rg -n "settingsWindow\\.center\\(" "$FILE" || true
echo "---- occurrences of center() ----"
rg -n "\\.center\\(" "$FILE" || true
# Check if settings window is restored/handled elsewhere (autosave names, window controller, etc.)
echo "---- occurrences of autosave/restore ----"
rg -n "autosave|FrameAutosave|setFrame|restore|center\\(" "$FILE" || trueRepository: kitlangton/Hex
Length of output: 2004
🌐 Web query:
NSWindow setFrameAutosaveName center() ordering restore frame autosave
💡 Result:
When using setFrameAutosaveName(:) on an NSWindow, AppKit automatically restores the window's last saved frame (position and size) upon assigning the autosave name [1][2][3]. The order of operations is critical. Because assigning an autosave name triggers an immediate reload of the frame data, any previous positioning calls (such as center) may be overwritten by the restored frame [2][3]. To ensure the window behaves correctly, follow this recommended sequence: 1. Configure the window's content and constraints [2]. 2. Call layoutIfNeeded to ensure the window has a valid size; calling center on a zero-sized window will not position it as expected [2]. 3. Call center to set the default initial position [2]. 4. Finally, call setFrameAutosaveName(:) [2]. By setting the autosave name last, you allow AppKit to override the default centered position with the user's previously saved frame, if one exists [2]. If no saved frame exists for that name, the window will remain in the position determined by your previous layout and center call [2]. Additionally, if you are managing multiple windows, ensure that window cascading is disabled to prevent the system from automatically offsetting the frame of new windows, which can conflict with autosave logic [4]. This is typically done via windowController.setShouldCascadeWindows(false) [4].
Citations:
- 1: https://developer.apple.com/documentation/appkit/nswindow/setframeautosavename(_:)
- 2: https://blog.xoria.org/cocoa-basics-2/
- 3: https://developer.apple.com/documentation/appkit/nswindow/frameautosavename-swift.property
- 4: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/WinPanel/Tasks/SavingWindowPosition.html
Frame autosave is being overridden by centering—swap the order.
settingsWindow.setFrameAutosaveName("Settings") causes AppKit to restore the saved frame immediately; the subsequent settingsWindow.center() then moves the window, defeating the restored placement. Center first so the autosaved frame (when present) wins.
Proposed fix
- settingsWindow.setFrameAutosaveName("Settings")
- settingsWindow.center()
+ settingsWindow.center()
+ settingsWindow.setFrameAutosaveName("Settings")📝 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
| settingsWindow.setFrameAutosaveName("Settings") | |
| settingsWindow.center() | |
| settingsWindow.center() | |
| settingsWindow.setFrameAutosaveName("Settings") |
🤖 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 `@Hex/App/HexAppDelegate.swift` around lines 126 - 127, The calls to
settingsWindow.center() and settingsWindow.setFrameAutosaveName("Settings") are
in the wrong order so the autosaved frame gets overridden by centering; swap
them so you call settingsWindow.center() first and then
settingsWindow.setFrameAutosaveName("Settings") (i.e., move the
setFrameAutosaveName call after the center call) to let AppKit restore the saved
frame when available.
Comment on lines
+1453
to
1458
| func cleanup() async { | ||
| endRecordingSession() | ||
| await resumeMediaIfNeeded() | ||
| stopObservingSystemChanges() | ||
| stopCaptureController(reason: "cleanup") | ||
| releaseRecorder(reason: "cleanup") |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
ls -la Hex/Clients/RecordingClient.swift
# Show the cleanup() function with surrounding context
python3 - <<'PY'
import re, itertools, pathlib
path = pathlib.Path("Hex/Clients/RecordingClient.swift")
text = path.read_text(encoding="utf-8")
m = re.search(r'func cleanup\(\) async\s*\{', text)
if not m:
raise SystemExit("cleanup() async not found")
start = m.start()
# print ~1200 chars around
print(text[start-200:start+1600])
PY
# Extract just the function body lines (best-effort line slicing)
nl -ba Hex/Clients/RecordingClient.swift | sed -n '1420,1520p'
# Search for observer registration/unregistration and the involved handlers
rg -n "stopObservingSystemChanges|resumeMediaIfNeeded|handleCaptureEnvironmentChange|cleanup\(\)" Hex/Clients/RecordingClient.swift
# Show implementations of the methods referenced in cleanup
for sym in stopObservingSystemChanges resumeMediaIfNeeded handleCaptureEnvironmentChange endRecordingSession stopCaptureController releaseRecorder; do
echo "==== $sym ===="
rg -n "func\s+$sym\b" -n Hex/Clients/RecordingClient.swift || true
done
# Print implementations of key symbols (line windows around first match)
for sym in stopObservingSystemChanges resumeMediaIfNeeded handleCaptureEnvironmentChange; do
line=$(rg -n "func\s+$sym\b" Hex/Clients/RecordingClient.swift | head -n1 | cut -d: -f1 || true)
if [ -n "${line:-}" ]; then
from=$((line-40))
to=$((line+160))
echo "==== $sym around lines $from-$to ===="
nl -ba Hex/Clients/RecordingClient.swift | sed -n "${from},${to}p"
else
echo "==== $sym not found ===="
fi
done
# Also look for any use of stopObservingSystemChanges in other files (optional but helpful)
rg -n "stopObservingSystemChanges" Hex | head -n 50Repository: kitlangton/Hex
Length of output: 826
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re, pathlib
path = pathlib.Path("Hex/Clients/RecordingClient.swift")
text = path.read_text(encoding="utf-8")
def show_match(name, pattern):
m = re.search(pattern, text)
if not m:
print(f"\n==== {name}: not found ====")
return
start = max(0, m.start()-400)
end = min(len(text), m.end()+1200)
print(f"\n==== {name} (context) ====")
print(text[start:end])
show_match("cleanup()", r"func\s+cleanup\(\)\s+async\s*\{")
show_match("stopObservingSystemChanges()", r"func\s+stopObservingSystemChanges\s*\(")
show_match("resumeMediaIfNeeded()", r"func\s+resumeMediaIfNeeded\s*\(")
show_match("handleCaptureEnvironmentChange", r"handleCaptureEnvironmentChange")
# Also print the exact cleanup body for diff precision
m = re.search(r"func\s+cleanup\(\)\s+async\s*\{\s*(.*?)\n\s*\}", text, flags=re.S)
if m:
body = m.group(1)
print("\n==== cleanup() body ====")
print(body.strip())
else:
print("\n==== cleanup() body: could not parse ====")
# Quick sanity: find any await sites between stopObservingSystemChanges and endRecordingSession
cleanup_start = text.find("func cleanup() async")
if cleanup_start != -1:
chunk = text[cleanup_start:cleanup_start+600]
print("\n==== cleanup() first ~600 chars ====")
print(chunk)
PY
# Find related usages/handlers in the file
rg -n "stopObservingSystemChanges|resumeMediaIfNeeded|handleCaptureEnvironmentChange|cleanup\(\)" Hex/Clients/RecordingClient.swift
# See if stopObservingSystemChanges is async or uses locks/flags
rg -n "stopObservingSystemChanges\s*\(" Hex/Clients/RecordingClient.swiftRepository: kitlangton/Hex
Length of output: 7342
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "enqueueCaptureEnvironmentChange\(" Hex/Clients/RecordingClient.swift
rg -n "notificationObservers|audioHardwareObservers" Hex/Clients/RecordingClient.swift
# Show the observer-install / callback wiring sections around those calls.
python3 - <<'PY'
import pathlib, re
p=pathlib.Path("Hex/Clients/RecordingClient.swift")
t=p.read_text()
# find occurrences of enqueueCaptureEnvironmentChange and print surrounding block
for m in re.finditer(r"enqueueCaptureEnvironmentChange\(([^)]*)\)", t):
s=max(0, m.start()-250)
e=min(len(t), m.start()+400)
snippet=t[s:e]
print("\n==== around enqueueCaptureEnvironmentChange ====")
print(snippet)
PY
# Also find the code where audioHardwareObservers are appended (common for listener installation)
rg -n "audioHardwareObservers\.append|notificationObservers\.append" Hex/Clients/RecordingClient.swiftRepository: kitlangton/Hex
Length of output: 6438
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
p=pathlib.Path("Hex/Clients/RecordingClient.swift")
t=p.read_text(encoding="utf-8")
# Extract handleCaptureEnvironmentChange function body (best-effort with simple brace counting)
start = t.find("private func handleCaptureEnvironmentChange")
if start == -1:
raise SystemExit("handleCaptureEnvironmentChange not found")
# find opening brace after signature
brace = t.find("{", start)
if brace == -1:
raise SystemExit("opening brace not found")
i=brace
depth=0
for j in range(brace, len(t)):
c=t[j]
if c=="{": depth+=1
elif c=="}":
depth-=1
if depth==0:
end=j+1
body=t[brace:end]
print(body)
break
else:
raise SystemExit("matching brace not found")
PY
# Focused searches for restart / arming calls inside the file
rg -n "handleCaptureEnvironmentChange|forceRestart|restart|arm capture|startCapture|stopCaptureController|releaseRecorder" Hex/Clients/RecordingClient.swiftRepository: kitlangton/Hex
Length of output: 4736
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "isObservingSystemChanges" Hex/Clients/RecordingClient.swift
rg -n "deferredCaptureRestartReason|invalidatePrimedState|clearMediaState|endRecordingSession" Hex/Clients/RecordingClient.swift | head -n 80Repository: kitlangton/Hex
Length of output: 1169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re
p=pathlib.Path("Hex/Clients/RecordingClient.swift")
t=p.read_text(encoding="utf-8")
def extract_func(name):
# find signature
m=re.search(rf'func\s+{re.escape(name)}\b[^(]*\([^)]*\)\s*(async\s*)?\{{', t)
if not m:
# try signatures without async/with labels
m=re.search(rf'func\s+{re.escape(name)}\b[^(]*\(', t)
if not m:
print(f"\n==== {name}: not found ====")
return
start=m.start()
brace=t.find("{", m.end()-1)
if brace==-1:
print(f"\n==== {name}: brace not found ====")
return
depth=0
for i in range(brace, len(t)):
if t[i]=="{": depth+=1
elif t[i]=="}":
depth-=1
if depth==0:
end=i+1
print(f"\n==== {name} (function body) ====")
print(t[brace:end])
return
print(f"\n==== {name}: matching brace not found ====")
for fn in [
"endRecordingSession",
"stopCaptureController",
"releaseRecorder",
"ensureCaptureControllerReady",
"primeRecorderForNextSession",
"invalidatePrimedState",
"clearMediaState",
]:
extract_func(fn)
PYRepository: kitlangton/Hex
Length of output: 2108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# show line-numbered context for the above funcs (smaller excerpts)
for start in 1240 1260 1270 1280 1290 1320 840 860 872 880 1080 1160; do
echo "==== context around $start ===="
sed -n "${start},$((start+60))p" Hex/Clients/RecordingClient.swift | cat
doneRepository: kitlangton/Hex
Length of output: 24727
Stop observing before the first await in cleanup()
RecordingClientLive.cleanup() currently awaits resumeMediaIfNeeded() before calling stopObservingSystemChanges(). During that await gap, the existing wake/device-change and CoreAudio listeners can enqueue handleCaptureEnvironmentChange(...) (via the 250ms debounce), which can restart/prime the capture engine/recorder (e.g. ensureCaptureControllerReady(...), primeRecorderForNextSession()) while teardown is still in progress.
Suggested fix
func cleanup() async {
+ stopObservingSystemChanges()
endRecordingSession()
await resumeMediaIfNeeded()
- stopObservingSystemChanges()
stopCaptureController(reason: "cleanup")
releaseRecorder(reason: "cleanup")
recordingLogger.notice("RecordingClient cleaned up")
}📝 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
| func cleanup() async { | |
| endRecordingSession() | |
| await resumeMediaIfNeeded() | |
| stopObservingSystemChanges() | |
| stopCaptureController(reason: "cleanup") | |
| releaseRecorder(reason: "cleanup") | |
| func cleanup() async { | |
| stopObservingSystemChanges() | |
| endRecordingSession() | |
| await resumeMediaIfNeeded() | |
| stopCaptureController(reason: "cleanup") | |
| releaseRecorder(reason: "cleanup") |
🤖 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 `@Hex/Clients/RecordingClient.swift` around lines 1453 - 1458, In
RecordingClientLive.cleanup(), stopObservingSystemChanges() must be called
before any await to prevent queued environment-change handlers from restarting
the capture path during teardown; reorder the calls so that
stopObservingSystemChanges() is invoked immediately after endRecordingSession()
and before await resumeMediaIfNeeded(), while preserving subsequent calls to
stopCaptureController(reason: "cleanup") and releaseRecorder(reason: "cleanup")
(also ensure any related listeners are deregistered in
stopObservingSystemChanges()).
Comment on lines
+386
to
+387
| let capturedURL = await recording.stopRecording() | ||
| audioURL = capturedURL |
Contributor
There was a problem hiding this comment.
Don't transcribe a stop result when the start phase never reached recording.startRecording().
state.isRecording becomes true before the recordingStart effect gets past sleepManagement.preventSleep(...), so a long-enough key hold can enter the transcription path even though no capture session ever started. In that case Hex/Clients/RecordingClient.swift:1132-1236 returns its ignored/stale-stop URL, and this flow still treats it as real audio. Track “capture actually started” separately, or make recording.stopRecording() return an explicit outcome so this branch can discard instead of transcribing.
🤖 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 `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 386 -
387, The code incorrectly treats any stop result as real audio because
state.isRecording flips true before recordingStart finishes; update the flow to
distinguish "capture actually started" from just the state flag: either add a
new boolean (e.g., captureStarted) set only after recording.startRecording()
completes in the recordingStart effect and check that before calling
recording.stopRecording()/using its URL, or change recording.stopRecording() to
return an explicit enum/result (e.g., .captured(URL) | .ignored) and only
assign/transcribe when the result is .captured; reference
recording.startRecording(), recording.stopRecording(), and state.isRecording to
implement the chosen approach so stale/ignored-stop URLs are discarded.
|
|
||
| let result = try await transcription.transcribe(capturedURL, model, decodeOptions) { _ in } | ||
|
|
||
| transcriptionFeatureLogger.notice("Transcribed audio from \(capturedURL.lastPathComponent) to text length \(result.count)") |
Contributor
There was a problem hiding this comment.
Mark the captured filename private in this log line.
capturedURL.lastPathComponent is still file-path-derived data and should use a privacy annotation.
Suggested fix
- transcriptionFeatureLogger.notice("Transcribed audio from \(capturedURL.lastPathComponent) to text length \(result.count)")
+ transcriptionFeatureLogger.notice("Transcribed audio from \(capturedURL.lastPathComponent, privacy: .private) to text length \(result.count)")📝 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
| transcriptionFeatureLogger.notice("Transcribed audio from \(capturedURL.lastPathComponent) to text length \(result.count)") | |
| transcriptionFeatureLogger.notice("Transcribed audio from \(capturedURL.lastPathComponent, privacy: .private) to text length \(result.count)") |
🤖 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 `@Hex/Features/Transcription/TranscriptionFeature.swift` at line 401, Replace
the direct transcriptionFeatureLogger.notice call with the unified HexLog helper
(use category .transcription or add it) and mark the captured filename as
private; specifically, when logging the file name from
capturedURL.lastPathComponent pass it with a privacy annotation (privacy:
.private) and keep the transcript/result text private as well if included.
Locate the existing transcriptionFeatureLogger.notice(...) invocation and swap
it to use HexLog(.transcription).notice(...) (or equivalent) and annotate
capturedURL.lastPathComponent with privacy: .private.
Comment on lines
+404
to
+406
| } catch { | ||
| transcriptionFeatureLogger.error("Transcription failed: \(error.localizedDescription)") | ||
| await send(.transcriptionError(error, nil)) |
Contributor
There was a problem hiding this comment.
Ignore cancellation here instead of sending .transcriptionError.
.cancel explicitly cancels CancelID.transcription, so a cooperative CancellationError from transcription.transcribe(...) will currently repopulate state.error after an intentional cancel. Treat cancellation as a no-op and only dispatch .transcriptionError for real failures.
Suggested fix
- } catch {
+ } catch is CancellationError {
+ return
+ } catch {
transcriptionFeatureLogger.error("Transcription failed: \(error.localizedDescription)")
await send(.transcriptionError(error, nil))
}📝 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
| } catch { | |
| transcriptionFeatureLogger.error("Transcription failed: \(error.localizedDescription)") | |
| await send(.transcriptionError(error, nil)) | |
| } catch is CancellationError { | |
| return | |
| } catch { | |
| transcriptionFeatureLogger.error("Transcription failed: \(error.localizedDescription)") | |
| await send(.transcriptionError(error, nil)) | |
| } |
🤖 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 `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 404 -
406, In the catch block after calling transcription.transcribe(...), detect
cancellation and treat it as a no-op: if the thrown error is a CancellationError
(or Task.isCancelled), do not call await send(.transcriptionError(...)) or
update state; otherwise log and send .transcriptionError as before. Update the
catch to check for CancellationError (or Task.isCancelled) and only dispatch
.transcriptionError for non-cancellation errors, referencing the existing
transcription.transcribe(...), .transcriptionError, and CancelID.transcription
flow.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Hex/Features/App/AppFeature.swift (1)
117-134: ⚡ Quick winRefresh permission state immediately and stop polling once it settles.
These branches wait one second before the first refresh, and then keep dispatching
.checkPermissionsfor the full polling window even after the permission has already flipped. That leaves the Settings badges stale longer than necessary and does extra work on the success path.Proposed adjustment
case .settings(.requestAccessibility): return .run { send in await permissions.requestAccessibility() + await send(.checkPermissions) + guard await permissions.accessibilityStatus() == .notDetermined else { return } // Poll for status change (macOS doesn't provide callback) for _ in 0..<10 { try? await Task.sleep(for: .seconds(1)) await send(.checkPermissions) + if await permissions.accessibilityStatus() != .notDetermined { return } } } case .settings(.requestInputMonitoring): return .run { send in _ = await permissions.requestInputMonitoring() + await send(.checkPermissions) + guard await permissions.inputMonitoringStatus() == .notDetermined else { return } for _ in 0..<10 { try? await Task.sleep(for: .seconds(1)) await send(.checkPermissions) + if await permissions.inputMonitoringStatus() != .notDetermined { 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 `@Hex/Features/App/AppFeature.swift` around lines 117 - 134, After calling permissions.requestAccessibility() or permissions.requestInputMonitoring() in the .settings(.requestAccessibility) and .settings(.requestInputMonitoring) branches, immediately dispatch await send(.checkPermissions) once so the UI refreshes right away, then enter the polling loop but stop early when the permission state has settled: in the loop call try? await Task.sleep(for: .seconds(1)); await send(.checkPermissions); and break out if permissions reports granted (e.g. via an existing permissions API like permissions.isAccessibilityGranted() / permissions.isInputMonitoringGranted() or by comparing prior/returned state) to avoid unnecessary further sends.
🤖 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 `@Hex/Features/History/HistoryFeature.swift`:
- Around line 62-67: play(url:) currently ignores AVAudioPlayer.play()'s Bool
result; if play() returns false the method should throw so callers don't hang
waiting for delegate callbacks. Modify play(url:) to capture the return value
(let started = player.play()), and if started is false throw a specific error
(e.g., add a PlaybackError enum with case playFailed(url: URL) or reuse an
existing error type). Keep setting player and delegate as-is, but after calling
play() check the Bool and throw PlaybackError.playFailed(url) when false so the
caller can handle the failure.
---
Nitpick comments:
In `@Hex/Features/App/AppFeature.swift`:
- Around line 117-134: After calling permissions.requestAccessibility() or
permissions.requestInputMonitoring() in the .settings(.requestAccessibility) and
.settings(.requestInputMonitoring) branches, immediately dispatch await
send(.checkPermissions) once so the UI refreshes right away, then enter the
polling loop but stop early when the permission state has settled: in the loop
call try? await Task.sleep(for: .seconds(1)); await send(.checkPermissions); and
break out if permissions reports granted (e.g. via an existing permissions API
like permissions.isAccessibilityGranted() /
permissions.isInputMonitoringGranted() or by comparing prior/returned state) to
avoid unnecessary further sends.
🪄 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: 8e59b899-550a-4076-a3f0-5cc974004a9b
📒 Files selected for processing (5)
Hex/Clients/RecordingClient.swiftHex/Features/App/AppFeature.swiftHex/Features/History/HistoryFeature.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Transcription/TranscriptionFeature.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Hex/Features/Transcription/TranscriptionFeature.swift
- Hex/Clients/RecordingClient.swift
Comment on lines
+62
to
67
| func play(url: URL) throws { | ||
| let player = try AVAudioPlayer(contentsOf: url) | ||
| player.delegate = self | ||
| player.play() | ||
| self.player = player | ||
| return player | ||
| player.play() | ||
| } |
Contributor
There was a problem hiding this comment.
Throw when AVAudioPlayer.play() returns false.
AVAudioPlayer.play() can fail without throwing. If that happens, Line 160 can wait indefinitely and the row remains in a playing state until manually stopped.
🔧 Proposed fix
class AudioPlayerController: NSObject, AVAudioPlayerDelegate {
+ enum PlaybackError: Error {
+ case failedToStart
+ }
+
private var player: AVAudioPlayer?
private let (playbackFinishedStream, playbackFinishedContinuation) = AsyncStream<Void>.makeStream()
func play(url: URL) throws {
let player = try AVAudioPlayer(contentsOf: url)
player.delegate = self
self.player = player
- player.play()
+ guard player.play() else {
+ self.player = nil
+ throw PlaybackError.failedToStart
+ }
}🤖 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 `@Hex/Features/History/HistoryFeature.swift` around lines 62 - 67, play(url:)
currently ignores AVAudioPlayer.play()'s Bool result; if play() returns false
the method should throw so callers don't hang waiting for delegate callbacks.
Modify play(url:) to capture the return value (let started = player.play()), and
if started is false throw a specific error (e.g., add a PlaybackError enum with
case playFailed(url: URL) or reuse an existing error type). Keep setting player
and delegate as-is, but after calling play() check the Bool and throw
PlaybackError.playFailed(url) when false so the caller can handle the failure.
This was referenced Jun 7, 2026
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
Fixes #234
Verification
git diff --checkcd HexCore && swift testxcodebuild -scheme Hex -configuration Debug buildKnown test infrastructure issue
xcodebuild test -scheme Hex -configuration Debugnow advances past the corrected Debug test-host path, but the Xcode test bundle still fails to link existing Swift package products becauseIssueReportingandConcurrencyExtrassymbols are unresolved. The application Debug build and the independentHexCoresuite pass.Follow-up
This PR intentionally does not attempt the larger AirPods route-churn redesign. A follow-up should preserve the normal Fast Mode warm-buffer path while avoiding eager in-process
AVAudioEngineteardown and rebuild during real route transitions such as AirPods handoff and Bluetooth A2DP-to-HFP switching.Summary by CodeRabbit
Bug Fixes
New Features
UI Improvements
Tests
Stack