Design-fidelity follow-ups: cosmetic alignment, sidebar breakdown, mic-test banner#19
Merged
Conversation
Pulls six values that drifted between the design prototype and the Swift handoff back to the prototype's numbers: - Welcome window 600x720 -> 580x700 - Status menu width 300 -> 320; "Help" item -> "Help / Welcome" - Welcome style-tile height 70 -> 64 - Recording window corner radius 18 -> 22 - Overview hero numeral 52pt -> 48pt - Classic waveform bar count 64 -> 56 Updates the affected tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ports the deferred sidebar enhancement from the design prototype: the "Total recorded" footer now shows a month-over-month delta line and, on hover, a popover with a lifetime breakdown — this month, last 7 days, average per day, time saved, and total sessions. All figures are computed from real UsageMetricsStore data. The prototype's "installed N days ago" row is omitted — the app does not persist an install date and fabricating one was not warranted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ports the deferred mic-test feature from the design prototype. A banner at the top of the Visuals tab captures live microphone audio and feeds it into the shared LivePreviewSampler, so every waveform preview reacts to the user's real voice. - MicTestCapture: AVAudioEngine input tap; RMS level, 64 time-domain samples, and 8 FFT bands (via the existing FFTProcessor). - LivePreviewSampler gains a mic mode that suppresses the synthetic envelope and publishes real frames. - MicTestBanner: live preview, state-driven copy, input level meter, and off/live/denied/error states with permission handling. Capture is visualization-only — nothing is recorded or persisted; the mic is released on stop, on error, and when the tab disappears. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces live microphone testing with real-time waveform visualization and level metering, alongside dashboard enhancements including a hover-activated breakdown popover for word count statistics. Welcome screen dimensions are refined, menu labels updated, and waveform styling constants tuned. ChangesMicrophone Test Feature
Dashboard and UI Refinements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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: 6
🤖 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/Views/Components/Waveform/MicTestBanner.swift`:
- Around line 224-229: The preview instantiates two separate LivePreviewSampler
instances so MicTestBanner and MicTestCapture don't share updates; create a
single sampler variable (e.g., let sampler = LivePreviewSampler()) in the
`#Preview` block and pass that same sampler to both MicTestBanner(sampler:
sampler) and MicTestCapture(sampler: sampler) so the live capture drives the
rendered waveform.
- Around line 19-23: The MicTestBanner SwiftUI view lacks main-actor isolation;
annotate the MicTestBanner type with `@MainActor` to ensure UI access is
main-thread safe — update the declaration of struct MicTestBanner (which holds
WaveformStyle, `@ObservedObject` var sampler: LivePreviewSampler, and
`@ObservedObject` var capture: MicTestCapture) by adding the `@MainActor` attribute
so the entire view and its lifecycle are main-actor isolated.
In `@Sources/Views/Components/Waveform/MicTestCapture.swift`:
- Around line 67-75: Add an explicit "captureRequested" Bool flag on the
MicTestCapture instance, set it true immediately before calling
AVCaptureDevice.requestAccess(for: .audio), and clear it in stop(); in the
requestAccess callback (the Task { `@MainActor` ... } block) only call
beginCapture() if granted && captureRequested is still true, otherwise set state
= .denied if !granted; apply the same guard to the other requestAccess callback
(the similar block around beginCapture() at the second occurrence) so late
permission responses cannot restart capture after stop() was called.
- Line 158: The code uses optional chaining on a non-optional property
`fftProcessor` when assigning `rawBands`; remove the invalid `?` and call
`fftProcessor.process(mono)` directly in `MicTestCapture.swift` (line with
`rawBands`) and either drop the fallback Array(repeating:0, count:8) or adapt to
the actual return type of `process` (e.g., handle an optional return from
`process` if needed) so the call compiles without optional chaining.
- Around line 41-42: smoothedLevel is mutated from the audio callback thread
while being read/reset on the main actor, but stateLock is unused—introduce
explicit locking around every access to smoothedLevel: wrap writes in the audio
callback with stateLock.lock()/defer { stateLock.unlock() } and wrap
reads/resets on the main actor (where smoothedLevel is inspected/reset) the same
way so all reads and writes use stateLock; keep the nonisolated(unsafe)
declaration but ensure stateLock protects smoothedLevel accesses (reference
smoothedLevel and stateLock) so there is no data race.
In `@Sources/Views/Dashboard/DashboardVisualsView.swift`:
- Around line 13-17: The DashboardVisualsView initializes MainActor-isolated
objects (LivePreviewSampler and MicTestCapture) from a non-isolated context;
annotate the DashboardVisualsView declaration with `@MainActor` so its init and
property wrappers (_sampler: StateObject, _micCapture: StateObject) run on the
main actor. Locate the DashboardVisualsView type (the struct/class that defines
init() shown) and add the `@MainActor` attribute to its declaration so the
LivePreviewSampler and MicTestCapture initializations occur under main-thread
isolation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1b553181-c2d1-4bed-bb6c-49c327c86dbe
📒 Files selected for processing (16)
Sources/App/AppDelegate+Menu.swiftSources/Design/LayoutMetrics.swiftSources/Views/Components/Waveform/ClassicWaveformView.swiftSources/Views/Components/Waveform/MicTestBanner.swiftSources/Views/Components/Waveform/MicTestCapture.swiftSources/Views/Components/Waveform/WaveformContainer.swiftSources/Views/Components/Waveform/WaveformPreviewHelpers.swiftSources/Views/Dashboard/DashboardHomeView.swiftSources/Views/Dashboard/DashboardView.swiftSources/Views/Dashboard/DashboardVisualsView.swiftSources/Views/WelcomeView.swiftTests/AppDelegate/AppDelegateMenuTests.swiftTests/AppDelegateExtensionTests.swiftTests/Design/LayoutMetricsTests.swiftTests/Views/Dashboard/DashboardVisualsViewTests.swiftTests/Views/WelcomeViewTests.swift
Comment on lines
+19
to
+23
| internal struct MicTestBanner: View { | ||
| let style: WaveformStyle | ||
| @ObservedObject var sampler: LivePreviewSampler | ||
| @ObservedObject var capture: MicTestCapture | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "MicTestBanner.swift" | head -20Repository: jtn0123/AudioWhisper
Length of output: 120
🏁 Script executed:
cat -n ./Sources/Views/Components/Waveform/MicTestBanner.swift | head -30Repository: jtn0123/AudioWhisper
Length of output: 1417
Annotate MicTestBanner with @MainActor.
This view is a UI component in Sources/**/*.swift and should be explicitly main-actor isolated.
💡 Proposed fix
-internal struct MicTestBanner: View {
+@MainActor
+internal struct MicTestBanner: View {As per coding guidelines, Sources/**/*.swift: Use @MainActor annotation for UI components in Swift.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| internal struct MicTestBanner: View { | |
| let style: WaveformStyle | |
| @ObservedObject var sampler: LivePreviewSampler | |
| @ObservedObject var capture: MicTestCapture | |
| `@MainActor` | |
| internal struct MicTestBanner: View { | |
| let style: WaveformStyle | |
| `@ObservedObject` var sampler: LivePreviewSampler | |
| `@ObservedObject` var capture: MicTestCapture |
🤖 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/Views/Components/Waveform/MicTestBanner.swift` around lines 19 - 23,
The MicTestBanner SwiftUI view lacks main-actor isolation; annotate the
MicTestBanner type with `@MainActor` to ensure UI access is main-thread safe —
update the declaration of struct MicTestBanner (which holds WaveformStyle,
`@ObservedObject` var sampler: LivePreviewSampler, and `@ObservedObject` var
capture: MicTestCapture) by adding the `@MainActor` attribute so the entire view
and its lifecycle are main-actor isolated.
Comment on lines
+224
to
+229
| #Preview("Mic test banner") { | ||
| MicTestBanner( | ||
| style: .classic, | ||
| sampler: LivePreviewSampler(), | ||
| capture: MicTestCapture(sampler: LivePreviewSampler()) | ||
| ) |
There was a problem hiding this comment.
Use one shared sampler in #Preview wiring.
The preview currently gives MicTestBanner one sampler and MicTestCapture a different sampler, so live capture updates won’t drive the rendered preview waveform.
💡 Proposed fix
`#Preview`("Mic test banner") {
+ let sampler = LivePreviewSampler()
MicTestBanner(
style: .classic,
- sampler: LivePreviewSampler(),
- capture: MicTestCapture(sampler: LivePreviewSampler())
+ sampler: sampler,
+ capture: MicTestCapture(sampler: sampler)
)🤖 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/Views/Components/Waveform/MicTestBanner.swift` around lines 224 -
229, The preview instantiates two separate LivePreviewSampler instances so
MicTestBanner and MicTestCapture don't share updates; create a single sampler
variable (e.g., let sampler = LivePreviewSampler()) in the `#Preview` block and
pass that same sampler to both MicTestBanner(sampler: sampler) and
MicTestCapture(sampler: sampler) so the live capture drives the rendered
waveform.
Comment on lines
+41
to
+42
| private nonisolated(unsafe) var smoothedLevel: Float = 0 | ||
| private let stateLock = NSLock() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n Sources/Views/Components/Waveform/MicTestCapture.swift | head -200Repository: jtn0123/AudioWhisper
Length of output: 7511
🏁 Script executed:
rg "stateLock" Sources/Views/Components/Waveform/MicTestCapture.swiftRepository: jtn0123/AudioWhisper
Length of output: 101
Synchronize smoothedLevel access with the existing stateLock across audio and main threads.
smoothedLevel is marked nonisolated(unsafe) to allow mutation from the audio callback thread (line 139) while reading/resetting on the main actor. The stateLock is declared but never used. Without synchronization, concurrent access creates a data race.
Proposed fix
func stop() {
teardownEngine()
+ stateLock.lock()
smoothedLevel = 0
+ stateLock.unlock()
level = 0+ stateLock.lock()
let smooth = smoothedLevel * 0.55 + target * 0.45
smoothedLevel = smooth
+ stateLock.unlock()Applies to lines 85, 151–152.
🤖 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/Views/Components/Waveform/MicTestCapture.swift` around lines 41 - 42,
smoothedLevel is mutated from the audio callback thread while being read/reset
on the main actor, but stateLock is unused—introduce explicit locking around
every access to smoothedLevel: wrap writes in the audio callback with
stateLock.lock()/defer { stateLock.unlock() } and wrap reads/resets on the main
actor (where smoothedLevel is inspected/reset) the same way so all reads and
writes use stateLock; keep the nonisolated(unsafe) declaration but ensure
stateLock protects smoothedLevel accesses (reference smoothedLevel and
stateLock) so there is no data race.
Comment on lines
+67
to
+75
| AVCaptureDevice.requestAccess(for: .audio) { [weak self] granted in | ||
| Task { @MainActor [weak self] in | ||
| guard let self = self else { return } | ||
| if granted { | ||
| self.beginCapture() | ||
| } else { | ||
| self.state = .denied | ||
| } | ||
| } |
There was a problem hiding this comment.
Prevent late permission callbacks from restarting capture after stop().
If the permission dialog resolves after the banner/tab is dismissed, the callback can still call beginCapture(), re-enabling mic unexpectedly. Gate async completion with an explicit “capture requested” flag that stop() clears.
💡 Suggested direction
final class MicTestCapture: ObservableObject {
+ private var shouldBeCapturing = false
@@
func start() {
guard state != .live else { return }
+ shouldBeCapturing = true
@@
case .notDetermined:
AVCaptureDevice.requestAccess(for: .audio) { [weak self] granted in
Task { `@MainActor` [weak self] in
guard let self = self else { return }
+ guard self.shouldBeCapturing else { return }
if granted {
self.beginCapture()
} else {
self.state = .denied
}
}
}
@@
func stop() {
+ shouldBeCapturing = false
teardownEngine()Also applies to: 83-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 `@Sources/Views/Components/Waveform/MicTestCapture.swift` around lines 67 - 75,
Add an explicit "captureRequested" Bool flag on the MicTestCapture instance, set
it true immediately before calling AVCaptureDevice.requestAccess(for: .audio),
and clear it in stop(); in the requestAccess callback (the Task { `@MainActor` ...
} block) only call beginCapture() if granted && captureRequested is still true,
otherwise set state = .denied if !granted; apply the same guard to the other
requestAccess callback (the similar block around beginCapture() at the second
occurrence) so late permission responses cannot restart capture after stop() was
called.
| let samples = Self.downsample(mono, count: 64).map { $0 * 0.9 } | ||
|
|
||
| // 8 frequency bands, ignoring the top ~35% of the range as noise. | ||
| let rawBands = fftProcessor?.process(mono) ?? Array(repeating: 0, count: 8) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the non-optional declaration and optional-chaining use are both present.
rg -n -C2 'private let fftProcessor = FFTProcessor\(\)|fftProcessor\?\.' Sources/Views/Components/Waveform/MicTestCapture.swiftRepository: jtn0123/AudioWhisper
Length of output: 501
Remove invalid optional chaining on fftProcessor.
Line 158 uses optional chaining on a non-optional value, which is a compile-time error.
💡 Proposed fix
- let rawBands = fftProcessor?.process(mono) ?? Array(repeating: 0, count: 8)
+ let rawBands = fftProcessor.process(mono) ?? Array(repeating: 0, count: 8)📝 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
| let rawBands = fftProcessor?.process(mono) ?? Array(repeating: 0, count: 8) | |
| let rawBands = fftProcessor.process(mono) ?? Array(repeating: 0, count: 8) |
🤖 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/Views/Components/Waveform/MicTestCapture.swift` at line 158, The code
uses optional chaining on a non-optional property `fftProcessor` when assigning
`rawBands`; remove the invalid `?` and call `fftProcessor.process(mono)`
directly in `MicTestCapture.swift` (line with `rawBands`) and either drop the
fallback Array(repeating:0, count:8) or adapt to the actual return type of
`process` (e.g., handle an optional return from `process` if needed) so the call
compiles without optional chaining.
Comment on lines
+13
to
+17
| init() { | ||
| let sharedSampler = LivePreviewSampler() | ||
| _sampler = StateObject(wrappedValue: sharedSampler) | ||
| _micCapture = StateObject(wrappedValue: MicTestCapture(sampler: sharedSampler)) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "DashboardVisualsView.swift" | head -5Repository: jtn0123/AudioWhisper
Length of output: 117
🏁 Script executed:
git ls-files | grep -E "(DashboardVisualsView|LivePreviewSampler|MicTestCapture)" | head -20Repository: jtn0123/AudioWhisper
Length of output: 224
🏁 Script executed:
cat -n Sources/Views/Dashboard/DashboardVisualsView.swiftRepository: jtn0123/AudioWhisper
Length of output: 14629
🏁 Script executed:
cat -n Sources/Views/Components/Waveform/MicTestCapture.swiftRepository: jtn0123/AudioWhisper
Length of output: 7511
🏁 Script executed:
rg -n "class LivePreviewSampler|struct LivePreviewSampler" --type swiftRepository: jtn0123/AudioWhisper
Length of output: 181
🏁 Script executed:
sed -n '1,50p' Sources/Views/Components/Waveform/WaveformPreviewHelpers.swift | cat -nRepository: jtn0123/AudioWhisper
Length of output: 2083
Annotate DashboardVisualsView with @MainActor.
The view initializes @MainActor-isolated objects (LivePreviewSampler and MicTestCapture) from a non-isolated context. Marking the view @MainActor ensures proper actor isolation and prevents concurrency boundary ambiguity when instantiating main-thread-only dependencies.
Proposed fix
+@MainActor
internal struct DashboardVisualsView: View {Per coding guidelines: Sources/**/*.swift requires @MainActor annotation for UI components.
📝 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
| init() { | |
| let sharedSampler = LivePreviewSampler() | |
| _sampler = StateObject(wrappedValue: sharedSampler) | |
| _micCapture = StateObject(wrappedValue: MicTestCapture(sampler: sharedSampler)) | |
| } | |
| `@MainActor` | |
| internal struct DashboardVisualsView: View { | |
| init() { | |
| let sharedSampler = LivePreviewSampler() | |
| _sampler = StateObject(wrappedValue: sharedSampler) | |
| _micCapture = StateObject(wrappedValue: MicTestCapture(sampler: sharedSampler)) | |
| } |
🤖 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/Views/Dashboard/DashboardVisualsView.swift` around lines 13 - 17, The
DashboardVisualsView initializes MainActor-isolated objects (LivePreviewSampler
and MicTestCapture) from a non-isolated context; annotate the
DashboardVisualsView declaration with `@MainActor` so its init and property
wrappers (_sampler: StateObject, _micCapture: StateObject) run on the main
actor. Locate the DashboardVisualsView type (the struct/class that defines
init() shown) and add the `@MainActor` attribute to its declaration so the
LivePreviewSampler and MicTestCapture initializations occur under main-thread
isolation.
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
Follow-ups from the round-2 fidelity audit of the design redesign. Three focused commits:
Cosmetic alignment — pulls six values that drifted between the prototype and the Swift handoff back to the prototype's exact numbers: Welcome window 600×720→580×700, status menu width 300→320, "Help"→"Help / Welcome", Welcome tile height 70→64, recording-window corner radius 18→22, Overview hero numeral 52pt→48pt, Classic bar count 64→56.
Sidebar lifetime-breakdown popover — the "Total recorded" sidebar footer now shows a month-over-month delta line and, on hover, a popover breaking down this month / last 7 days / avg per day / time saved / sessions. All figures from real
UsageMetricsStoredata. (The prototype's "installed N days ago" row is omitted — the app doesn't persist an install date and fabricating one wasn't warranted.)"Test with my voice" mic-test banner — ports the deferred Visuals feature: a banner that captures live mic audio (
AVAudioEnginetap → RMS level + 64 samples + 8 FFT bands) and feeds the sharedLivePreviewSamplerso every waveform preview reacts to the user's real voice. Off/live/denied/error states with permission handling. Capture is visualization-only — nothing recorded; the mic is released on stop/error/tab-dismiss.Test plan
swift build+swift build --build-testscleanswiftlint --strictclean🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Style
Tests