Ambient recorder, Observation/Swift 6 migration, and an insights dashboard - #866
Closed
DhavalFatnani wants to merge 47 commits into
Closed
DhavalFatnani wants to merge 47 commits into
DhavalFatnani wants to merge 47 commits into
Conversation
Ad-hoc signing produces a new code signature on every build, and macOS records
privacy grants (Accessibility, Screen Recording, Microphone) against the
signature rather than the path. Every rebuild therefore looked like a new app
and silently lost its permissions — the symptom being VoiceInk asking for
Accessibility while System Settings showed it already enabled, with several
stale TCC records accumulated behind one visible row.
- Add scripts/make-local-signing-cert.sh to create a stable self-signed
code-signing identity, and repair the trust setting when a certificate
already exists but is not yet a valid identity.
- `make local` re-signs the built app with that identity when present. It is
done after xcodebuild rather than as a build setting because the app target
defines CODE_SIGN_IDENTITY[sdk=macosx*], which outranks both the xcconfig and
the command line — Xcode silently falls back to ad-hoc otherwise.
- Add com.apple.security.cs.disable-library-validation to the local
entitlements. A self-signed certificate carries no Team ID, so the hardened
runtime refuses to load the app's own dylibs ("different Team IDs") and the
app dies at launch. Release builds keep library validation enabled.
- Pass -skipPackagePluginValidation / -skipMacroValidation. The mlx-swift
dependency ships a CudaBuild plugin that Xcode blocks pending a one-time GUI
trust click, which no command-line build can satisfy — a clean `make local`
failed with Error 65 for anyone who had not already clicked it.
Foundation work behind the surface changes that follow. Design system - AppTheme gains Typography, Spacing, Motion and Recorder token groups. Typography is built on relative text styles, so the app responds to Dynamic Type; 537 hardcoded .font(.system(size:)) call sites previously prevented it. - AppCardBackground and AppMaterialCardBackground now use continuous corners. They were circular, which reads wrong next to native macOS controls, while MetricTintBackground in the same file already used continuous. - Split the mis-wired sidebar colour token: AppTheme.Sidebar.audio (pink) was applied to the History row while Audio fell through to fallback grey. Window - Remove the fixed 950pt width. minSize/maxSize both pinned it, the root view hard-set .frame(width:), and the scene used .windowResizability(.contentSize) — even though DashboardLayout already computes a width from the geometry. Now 820x560 minimum and freely resizable; a restored frame keeps its size rather than being snapped back to the default. - Wire Cmd-, through CommandGroup(replacing: .appSettings). The only binding previously lived inside the menu-bar extra's menu, so it fired only while that menu was open. Shell - Migrate ContentView from a hand-rolled HStack to NavigationSplitView, and AppSidebar to a List. This is what makes .searchable and .toolbar work at all in the detail views; without a navigation container they render nowhere. The coloured icon tiles are preserved, with selection, hover and keyboard navigation now inherited from the List.
History - Replace the Form-with-a-Section-per-row list with a List. Form does not virtualize, and every expanded row nested a ScrollView inside the outer scroll view, so a trackpad gesture over a transcript trapped the scroll. - Move selection from Set<Transcription> to Set<PersistentIdentifier>. Holding live SwiftData models in a Set retained deleted objects and relied on their identity staying stable. - Adopt .searchable with a 250ms debounce. Search previously re-ran a full predicate fetch on every keystroke. - Add keyboard support: arrows, Cmd-A, Delete, space to expand, Cmd-C, context menus. Replace the "Load More" button with infinite scroll, and the hand-built empty states with ContentUnavailableView. - Add transcript pinning and tags, with pinned items in their own section. Pinned is a separate section rather than a sort key because SortDescriptor requires a Comparable value and Bool is not one. Audio - Redraw the waveform as a single Canvas driven by TimelineView(.animation), replacing 200 individual bar views re-evaluated ten times a second from a published Timer. Playback completion now comes from AVAudioPlayerDelegate, which also removes a Timer being invalidated from deinit off the main thread. - Fix the waveform sampling: it stored abs(channelData[0]) — the first frame of each window — into an array named maxValues, producing a point sample rather than an envelope. It now takes the true peak across the window. - Add drag-to-select trimming with audio + transcript clip export. Recorder - Replace the flat opaque Color.black with a material, rim light and shadow, and move every literal Color.white to AppTheme.Recorder tokens. - Extract RecorderPresentation. Mini and Notch independently re-derived the same display state from the same inputs, and had already drifted. - Replace the Timer-driven dot animation with PhaseAnimator and the retained repeatForever spinner with TimelineView, and swap the DispatchWorkItem hover debounce for .task(id:). Also adds a Cmd-K command palette with fuzzy matching over destinations, modes and recent transcripts, and deduplicates two byte-identical dashboard callout cards into DashboardCalloutCard.
Migrates all 41 ObservableObject types to @observable and turns on Swift 6 language mode. Zero @published, @StateObject, @ObservedObject or @EnvironmentObject remain. Combine couplings that needed redesign rather than translation - ModeFormWarmupStore subscribed to objectWillChange on three services. It now wraps its snapshot build in withObservationTracking via a new re-arming ObservationBridge helper, which is strictly narrower: it wakes only on the properties the snapshot actually reads. - AIService and AIEnhancementService called objectWillChange.send() because their state lives in the keychain and UserDefaults, where Observation cannot see it. Replaced with an explicit externalStateRevision counter that the derived accessors read. - UpdaterViewModel used assign(to: &$x), which only works with @published, and RecorderPanelShortcutManager consumed $isRecorderPanelVisible.values. Both rewritten against Observation. Latent bugs surfaced by strict concurrency - Eleven deinit bodies touched main-actor state from a nonisolated context. @observable turns stored properties into computed ones, so lifecycle handles are now explicitly nonisolated(unsafe) or @ObservationIgnored. - FluidAudioStreamingProvider held an NSLock across await boundaries in nine places; converted to scoped withLock. - WhisperModelManager's KVO progress callback captured mutable locals while firing on an arbitrary queue; moved into a lock-guarded LockedValue box. - Recorder passed a non-Sendable Notification into a Task; it now extracts the AudioDeviceID in the observer. - AudioCleanupManager's Timer captured a non-Sendable ModelContext; replaced with a structured main-actor task, so the context never crosses isolation. Where a framework type is genuinely thread-safe but unannotated — NSCache, NSXPCConnection, XPC reply blocks, UserDefaults — the escape hatch is used deliberately and commented rather than applied blanket. Also removes a dead `import Charts` (Swift Charts is unused; all plotting is hand-rolled) and a no-op DashboardAmbientBackground that rendered Color.clear.
Three presentation changes on the recorder chrome, all surfacing state the panel already had. Rim + width grammar - RecorderRimState derives one colour from the recording state — neutral idle, red recording, amber processing — drawn as the panel's border plus a soft outer bloom, so state is readable peripherally without occupying layout. On the notch the stroke is masked to the visible lower edge, since a full stroke would draw a line across the display cutout. - RecorderWidthClass gives each width exactly one meaning: compact idle, standard recording, wide for something to read, conversation for the assistant. Width previously changed only for transcript and assistant, so the same width could mean two different things. The notch keeps its control column at a fixed width so the buttons do not squash to zero while the pill animates back to the cutout. Hold vs toggle - Push-to-talk and toggle rendered identically, leaving no way to tell whether releasing the key would stop the take. Hold modes now draw an outer ring — "press and hold" when idle, sustained pressure while recording — and toggle keeps the plain disc. Inline mode row - Replace the hover popover with a row of the first four enabled modes, expanded inline, each carrying its ⌥-number badge. RecorderPanelShortcutManager has always bound ⌥1–9 to modes while the panel is visible, and nothing on screen said so.
Marking AudioDeviceManager @mainactor during the Observation migration made its CoreAudio property listener inherit main-actor isolation. That listener is invoked by CoreAudio on its own dispatch queue, so the runtime asserted the isolation and trapped in _swift_task_checkIsolatedSwift on every device change — plugging or unplugging headphones crashed the app. Make setupDeviceChangeNotifications nonisolated so the C callback does not inherit isolation, and hop to the main actor explicitly inside it. Adds a nonisolated logger for the one error path that setup can hit. Audited the other C-callback sites: CoreAudioRecorder is @unchecked Sendable and ShortcutMonitor is unisolated, so neither has the same problem.
…ne row Input health and context answer the same question at different moments: can I trust this take? Health is about the signal coming in, context is about what goes out with it. They share one row split by a divider, each side owning a colour family (green/red for health, blue for context) that nothing else in the recorder uses, so the split reads without labels. Health - RecorderInputHealthMonitor samples the existing audio meter at 10Hz while recording and classifies it as clear, quiet or clipping. Deliberately slow to alarm and quick to clear: a single loud syllable is not clipping, and a pause for breath is not a dead microphone. The visualizer already moved with volume, but a dead mic and a quiet room looked identical until the transcript came back empty. - The active input device is named only when the input looks unhealthy, since the wrong microphone is usually the cause. Context - The engine already captured selection, clipboard and screen text per take via RecordingContextSnapshotStore, and none of it was ever shown — you could not tell whether your selected paragraph was about to be sent to a cloud provider. The store is now @observable so chips appear as each capture task lands, and RecorderStateProvider exposes a summary to both panels. The strip only renders when it has something non-obvious to say. A healthy take with nothing attached shows no strip at all and the panel stays compact — degradation earns the space.
Density is judged per take from live conditions rather than chosen in settings. RecorderDensityJudge maps conditions to minimal / standard / expanded: - realtime transcription on, or a take past ~45s -> expanded - health problem, attached context, unfamiliar mode -> standard - healthy, nothing attached, familiar mode -> minimal It ratchets: density can grow mid-take but never shrinks mid-take, because a panel collapsing while you are reading it is worse than one briefly larger than it needs to be. The ratchet resets when the take ends. RecorderModeFamiliarity counts uses per mode so a newly created mode shows more of itself for its first few takes and then gets out of the way. Density is evaluated on the same 10Hz loop that already samples input health, so this adds no additional polling. deviceChangedDuringTake is wired through the judge but always false for now — the recorder handles the switch internally and does not yet report it upward.
The transcript used to be pasted and the panel vanished. If it went into the wrong app, picked the wrong mode, or mangled a name, the only recourse was a shortcut the user may never have bound. LastTranscriptionService already implemented retry and paste-original; they simply had no visible home. The peek follows the paste rather than gating it — text lands immediately and this is an undo affordance, not a confirmation step. It auto-dismisses after 4s, and hovering pauses the countdown so reading it does not cost the chance to act. - Undo sends Cmd+Z to whatever now has focus. Leaning on the target app's own undo stack is what makes it feel native, and also why it cannot be guaranteed: an app without undo support, or one typed into since, will not restore cleanly. Documented at the call site rather than papered over. - Retry re-runs the same audio through the current mode without re-speaking. - Original/Enhanced toggles only when enhancement actually changed the text. The peek is skipped entirely when auto-send is enabled, since the text is already committed elsewhere and offering undo there would be misleading. Peek actions live on RecorderStateProvider rather than being threaded as four more callbacks, so the window managers stay unaware of the peek entirely.
… default Four defects found by testing the previous commits. Result peek never appeared - The presenter was guarded on `activePipelineTranscriptionID == transcriptionID`, copied from the sibling hooks. But it fires after the paste completes, by which point the pipeline has finished and cleared that ID, so the guard rejected every peek. Removed rather than replaced: delivery only reaches paste for a take whose status is already .completed. Input health always reported clipping - AudioMeter reports normalized 0…1, not dBFS — Recorder maps −60…0 dB onto that range and EMA-smooths it. The thresholds were written in dBFS, so `peak >= -1.5` was trivially true on every sample. Now 0.96 / 0.10 in the meter's own units, with the dB equivalents noted. Mode row was clipped and unusable - The expanded row was drawn inside a panel still fixed at the compact 184pt, so the chips ran off the right edge. The row now widens the panel while open — 330pt for the mini recorder, a larger side expansion for the notch — leaving the width grammar untouched for every other state. Both host windows were already large enough (540pt and notch+500pt), so no window changes needed. Hold affordance never showed - Defaulted to .toggle, but ShortcutMigration.migrateShortcutMode returns .hybrid when nothing is stored, which is what the app actually runs on a stock configuration. Anyone who had not opened the setting saw the toggle disc while behaving as push-to-talk.
…ealth Undo did nothing - Clicking a peek button makes the recorder panel key, which pulls focus off the app the text landed in, so the synthesized Cmd+Z went to the panel. The peek now carries the target app's bundle identifier, captured at paste time, and both actions reactivate it and let the activation settle before sending keys. Retry did nothing visible - It called LastTranscriptionService.retryLastTranscription, which only copies to the clipboard. That is right for a shortcut fired from anywhere and wrong for a button sitting next to text the user expects to be replaced. The peek now re-transcribes and pastes into the original target, reporting a missing audio file or model rather than failing silently. Input health never left "clear" - 0.90 / 0.16 rather than 0.96 / 0.10. The EMA smoothing (0.6 old / 0.4 new) pulls sustained peaks well below their instantaneous value, so 0.96 was effectively unreachable and clipping never fired. Mode row still clipped - 470pt for the mini recorder and 230pt side expansion for the notch, up from 330 and 150. Both stay inside what the host windows already reserve. Chips get slightly more padding and a bounded name width so long mode names truncate instead of squeezing their neighbours out. Hold affordance was invisible - The ring was 1pt and flush against the disc. It is now detached with a real gap and a smaller inner disc, so a hold control reads as something you press into rather than a switch you flip.
…space Undo still did nothing - The previous fix reactivated the target app before sending Cmd+Z. That was the wrong approach: macOS 14+ restricts cross-app activation, so the call was quietly ignored and focus stayed on the panel. The panels now refuse key focus entirely unless the assistant is on screen and expecting typed input, so clicking a peek button never pulls focus off the editor and the keystroke lands where it should. Removed the reactivation helper. Retry produced the text twice - It pasted the new transcription without removing the first, so the sentence appeared duplicated. Retry now undoes before pasting, making it a replace. Note the retried text will usually be identical: it re-runs the same audio through the same mode, so it is a recovery path, not a reroll. Mode row squeezed every other control - The row shared an HStack with the audio visualizer, which competed for the same space and collapsed the controls. The visualizer now yields while the row is open. Input health thresholds are still not firing - Rather than guess a third time, RecorderMeterDebug logs the observed peak maximum and average minimum per take so the next calibration is measured.
…plit notch row Input health, rebuilt from logged data - Measured on real hardware: whisper peaks 0.41–0.47, normal speech 0.75–0.88, shouting 0.91–1.00. Both earlier attempts failed for the same structural reason — they required N *consecutive* samples over a threshold, and speech is bursty, so peaks touch the top briefly and never sustain. - Now counts the proportion of hot samples in a 3s window: 35% of samples above 0.90 reads as too loud, which is what shouting actually looks like. Renamed from "clipping" to "too loud", since true clipping is rare and shouting is the case worth catching. - "Quiet" is gone. A whisper still transcribes fine, so flagging it was wrong. Replaced with "not hearing you", which fires only when there is no signal at all. The previous quiet metric was also useless: average min was 0.000 on every take, because every take opens with silence. Retry now re-enhances instead of re-transcribing - Re-transcribing was the obvious reading and the wrong one: the same audio through the same model returns the same words, so the button appeared to do nothing. It now re-runs AI enhancement over the transcript, which samples afresh and can actually produce a better result, and says so when the mode has no enhancement configured. Notch mode row no longer hides under the camera - The row was drawn in the left column only, so half of it disappeared beneath the notch. Chips are now split across both sides of the cutout, and the visualizer yields to them while the row is open. Hold affordance removed - Both modes still read identically to the user after two attempts. A 21pt control has to carry recording state and cannot also carry the interaction model; the distinction belongs elsewhere, or nowhere.
Predicted wait
- ModelPerformanceSummary has been recording averageSpeedFactor and
averageProcessingDuration per model all along and nothing read them. A 40s take
on a model that runs at 3x realtime is done in roughly 13s, so the panel now
says that instead of showing an indeterminate spinner. Transcribing and
enhancing are also labelled separately — they are different waits.
- Requires at least 3 past sessions for that model before promising anything,
and shows its provenance ("Parakeet · 3.1x realtime") so the number is not
mysterious. Progress caps at 97%: a bar sitting at 100% while still working
reads as stuck.
Ambient — the border is the instrument (Direction A)
- A third recorder style with no panel at all. A full-screen click-through
window draws the display border, which never takes focus or intercepts a
click.
- The governing rule is that the frame never says two things at once. An arbiter
picks one state by priority — input failing, then working, then listening —
and the whole border says only that, so position carries no meaning and there
is no legend to learn.
- Two properties only: colour for what is happening, thickness for how loud you
are. Progress is the exception and uses motion, a light completing one lap of
the perimeter.
- Amplitude response is damped asymmetrically (fast attack, slow release) so the
border does not strobe, and Reduce Motion flattens it to a static tint with no
breathing and no travelling light — a pulsing full-screen border is close to a
migraine trigger.
Direction B, the summoned panel, stays parked as agreed.
…mation Silence auto-stop - Ends a take that has gone quiet, for the "walked away with the recorder running" case that otherwise sends minutes of room tone to a paid API. - Warns before acting: 8s of silence starts a 5s countdown shown in the Signal Strip, with a Keep recording button. Any sound at all clears it outright — stopping someone mid-thought while they pause to think would be worse than the problem being solved. Slide to cancel - Drag the pill sideways past 90pt to abandon the take, replacing a double-press of Escape that nothing on screen advertised. Resistance grows with distance (square-root damping) so the pill visibly fights back, which is what makes the gesture discoverable by accident rather than only by being told. Vocabulary confirmation - WordReplacementService now reports which terms actually fired, and the result peek shows the count. Replacements have always run on every transcript with no feedback, so people added dictionary entries and never learned whether they worked. Terms matching through several variants are reported once.
The first version was correct and ugly: a hard-edged rectangular stroke with a drop shadow, which reads as a rendering error rather than something glowing. Three things were wrong with it. - Sharp rectangle corners fought the display's rounded corners. Now a continuous rounded rectangle approximating the bezel radius, so the light hugs the screen. - A 2–9pt hard line drawn on top of the user's work. Now a 26–62pt band pushed half off-screen and heavily blurred, so what is visible is only its inward falloff — light seeping in from the bezel. A faint 1pt line at the edge gives that light an origin. - systemGreen / systemRed / systemOrange are tuned to sit inside controls against a known background; spread across a whole display edge they read as a coloured rectangle. Replaced with lighter, less saturated tones. Also adds a per-state intensity so the common case stays calm: listening is noticeably dimmer than a problem, which earns the right to be loud. The progress comet is now a soft wide tail with a brighter core rather than a masked rectangle segment, so it reads as a moving light.
…een edge Ambient lit the whole display perimeter equally, which wastes the one place a notched Mac's user is already looking. On a notched display the cutout is now the focal point and the screen edge drops back to a supporting wash at 35%. The notch gets a form the screen edge cannot have. It is physically black and unlit, so a halo hugging its contour reads as the hardware itself glowing — where the same treatment on a screen edge always has content behind it. The halo traces the existing NotchShape, so it follows the cutout rather than boxing it. Scale is tuned separately: 9–25pt rather than the edge's 26–62pt, since the same bloom that reads as a soft wash across a display would swallow an object this small. Progress laps the cutout instead of the perimeter. The circuit is far shorter, so the same elapsed time reads as a much more legible rate of travel — a light crawling around a whole 15" display barely appears to move.
Halo was too bright - Bloom opacity 0.45 rather than 0.9, contour 0.40 rather than 0.85, and width 6–17pt rather than 9–25pt. At full strength the crisp contour outlined the cutout like a sticker stuck over it instead of lighting it. A panel for ambient, without becoming a panel - A capsule with a fill and a border would be the mini recorder wearing a glow, and would undo the premise: ambient exists so nothing sits on top of the work. So the caption has no container at all — illuminated text under the notch, lit by the same light the border is made of, as though the halo condensed enough to be read. Buttons are text with a hover underline rather than chips. - It grows out of the notch instead of opening like a window, so it reads as the light thickening rather than a new surface arriving. - Words appear only where light cannot carry the meaning, in priority order: result, then silence countdown, then an input problem, then what context is being sent. Everything else stays wordless, which is what keeps a caption meaningful when one does appear. - The context line retires after 3s. It is an acknowledgement, not a status field. Click-through, precisely - The window no longer ignores mouse events outright, since the caption has buttons. A hosting-view subclass returns nil wherever the SwiftUI content is not hit-testable, so every click outside the caption still falls through to the app underneath. Without it a display-sized window swallows every click on the machine.
Border thickness alone is a poor carrier for a voice. It has one dimension and no memory, so a steady tone and a sentence full of stresses look identical. The trace has both. On a notched Mac it hangs off the cutout's straight bottom edge — the one line the hardware already draws — so the waveform appears to be emitted by the notch rather than floating near it. On a display without a notch it sits under the top edge. - Drawn in Canvas as two passes: a wide blurred stroke that makes it read as light, and a crisp thin one for definition. - Mirrored around the centre line and smoothed with quadratic segments, so speech reads as a waveform rather than a sawtooth of raw samples. - Older samples fade toward the leading edge, so the trace streams out of the left instead of being visibly clipped by it. Sampling moves to 30Hz for smoothness, but the health monitor is still fed every third tick. Its windows are counted in samples and calibrated at 10Hz, so changing the rate would have silently rescaled its timing — the 3s window would have become 1s and the thresholds we measured would be wrong again. The caption drops 16pt to clear the trace rather than overlapping it.
The first version stroked a *closed* mirrored path, so what actually got drawn was the outline of a lens — a diagram of a waveform rather than a waveform. It also faded by tapering geometry toward the leading edge, which turned it into a lopsided wedge instead of something streaming. Rebuilt around a different anchor. The newest sample now sits at the centre and older ones are pushed outward in both directions, so the voice appears to be emitted by the notch and ripple away from it. Two problems solve themselves: it is symmetric under a centred cutout, and both ends taper to nothing on their own, so nothing is ever visibly clipped. It is filled rather than stroked, in three passes ordered by how far the light travels — a wide bloom outside the shape, a body lit brightest along its own centre line, and a hairline on the upper and lower contours. The rim is what lets you read individual syllables; without it loud passages merge into one bright mass. Meter values cluster low — normal speech averages around 0.45 — so a linear mapping wasted most of the available height. A root curve lifts the range people actually speak in without letting shouting run off the top, and a small resting amplitude keeps the shape present in silence so it reads as listening rather than as something that failed to draw. Wider and taller to match, and the caption drops another 10pt to clear it.
Every one of these shipped in this session as "working" and several were not.
The health monitor alone was wrong twice: once comparing normalized 0…1 meter
values against dBFS thresholds, once requiring consecutive samples over a line
that bursty speech never crosses twice in a row. Both were found by hand, and
both are one assertion each.
46 tests across the four types, weighted toward the cases that actually broke:
RecorderInputHealthMonitor — a whisper is not a problem, one loud word is not
shouting, sustained shouting is, and the window has to fill before it judges
anything so the opening silence of a take is not a dead microphone.
RecorderDensityJudge — the ratchet, which is the only reason the type exists:
density climbs mid-take and never shrinks, releases on endTake, and does not
carry its clock into the next take.
RecorderSilenceWatch — mostly that it does *not* fire. Cutting someone off
mid-thought is worse than the abandoned-recorder case it exists for.
RecorderProcessingEstimate — when it declines to predict, and the 0.97 cap,
which is load-bearing: overrunning the estimate is normal and a bar parked
at 100% reads as hung.
RecorderProcessingEstimate needed two seams to be testable at all — it looked up
history through a singleton and called Date() inside tick(). Both are now
injectable, with the old signatures preserved as the default path.
`make test` runs the suite. It has to sign the way `make local` does, because
the test host is the real app bundle and the shipping entitlements demand a
provisioning profile.
…ript Two problems, one cause. The trace read as a widget parked on the wallpaper rather than part of the ambient light, and the reason was measurable: with the notch present the edge wash ran at `intensity * 0.35`, so while listening the frame sat at 0.19 and the trace at 0.55. Nearly three times brighter. They were never going to look like the same light, and there was a band of unlit screen between them besides. The crest replaces the trace. It is the same waveform, folded into the frame instead of floating under it — a band hanging off the display's actual top edge, following the notch silhouette across the middle and easing out to the flat bezel over the shoulders, with its lower contour as the waveform. Where the voice does not reach it settles to the depth of the side-edge wash, so the top flows into the corners and down the sides with no seam. Speaking makes the light bulge downward out of the cutout and spread along the bezel. The part of the old version that worked survives: newest sample at the centre, older pushed outward both ways, so the voice appears emitted by the hardware. Intensity is up across the board — listening 0.55 → 0.72, working 0.75 → 0.88, and the edge multiplier 0.35 → 0.62. Live transcription arrives on the same principle rather than as a panel, which is the one thing this surface cannot have. The crest fades older audio outward from the notch; the transcript fades older words the same way. The word being spoken is at full brightness and the tail recedes to nothing, so the sentence appears to condense out of the light as you say it. Head truncation means words leaving on the left are already invisible when they are cut. It takes the caption slot at the lowest priority. The transcript runs for the whole take, so a problem, a countdown or a result has to be able to interrupt it — which is the same one-thing-at-a-time rule the frame itself follows. Per-word brightness is an AttributedString rather than an HStack of Text views, so the line still measures, wraps and truncates like ordinary text. The caption container animates on the *kind* of caption now, not its contents: keyed on the value, every word of the transcript re-ran the grow-out-of-the-notch transition.
…ible transcript Four changes, all following from the crest now being the frame's top edge. **The cutout walls are filled.** The silhouette eased back to the flat bezel over 130pt, which is nothing like the shape of a notch — the band had already climbed away from the glass by the time it passed the cutout's corner, leaving an unlit wedge either side. The ease is now 14pt, roughly the wall itself, so the light reaches the cutout's edge and turns the corner instead of cutting it. **Processing gets the modulator.** The crest stops being a live meter and becomes the take being read back: the whole waveform laid out from the notch outward, the finished portion lit, the remainder waiting in outline, and a playhead travelling out to each corner. Progress becomes distance along your own recording rather than an abstract bar — and the yellow now has something to do besides sit there. This is honest only when there is a real prediction behind it, so it needs RecorderProcessingEstimate to have enough history. Without one there is nothing truthful to sweep, and the indeterminate lap stays. Determinate when we know, indeterminate when we do not. Buckets take the peak, not the mean. Averaging a minute of speech flattens it into a mound that no longer looks like anything was said, and one shouted word in a quiet take would disappear entirely. **The transcript is readable.** It glowed the whole line and let old words drop to 10% white, which over a bright window turned to fog. Text sitting on arbitrary content needs a *dark* shadow before a coloured one — contrast is what makes glyphs sharp — and the fade needs a floor, because a word too dim to read still occupies the line and the eye still tries. Structure is now three tiers rather than a ramp: the newest word carries the state colour, the recent tail is white, older words recede to a readable grey. It reads as a sentence assembling itself and shows where the machine currently is. **Modes are switchable.** The panel hides them behind a hover-to-expand icon; ambient has no chrome to hang that off, and a hover target parked over the user's work would swallow clicks. So it inverts: the modes are legible for the whole take and gone the rest of the time. The shortcut is printed beside each name, so the row teaches ⌥1–9 every time you record. Being visible only during a take is also what makes it safe to accept clicks at all. Caption and mode strip now share a stack, so neither needs to know the other's height.
…t shows up **The processing crest was mostly invisible, and that was my error.** It only drew when RecorderProcessingEstimate had a prediction, which needs three prior takes on the same model. Most sessions have none, so the yellow state fell back to the indeterminate lap and the modulator never appeared at all. It now always replays the take. With history behind it the sweep is the real predicted progress; without, it is a repeating outward sweep. An honest indeterminate sweep beats an empty screen — it just does not get to claim a duration, which is why the caption's countdown stays absent in that case. The wait is also now stated in words. "Transcribing · ~7s left" with the model and its measured speed underneath, so the number is attributable rather than mysterious. That data has been recorded all along and ambient never showed it. **The result no longer lands on a dark screen.** A new settled state keeps the light on while the peek is offered, with the crest fully lit and still — the finished shape of what you just said. The peek itself grew into the one caption allowed to be more than a line: duration, word count, mode, whether enhancement ran, how many dictionary terms fired, and a show-original toggle when enhancement actually changed something. All of it was already on RecorderResultPeek and none of it was being shown. **The take bar** adds the four things the panel had and ambient did not: elapsed time, whether the text will be enhanced before it lands, the input device when the signal is failing, and a way out. Enhancement is the consequential one — it changes what comes out of the take and was entirely invisible. **Modes are readable.** The inactive floor was 0.45 white with no dark shadow, which over a bright window was not readable at all, and a control you cannot see is not a control. Now 0.78 with the same dark-shadow-first treatment the transcript needed, and the row sits on its own bloom. **The caption moved up ~40pt.** It was clearing the crest's *maximum* reach, so it floated unattached over dead screen most of the time. Measuring against a typical crest instead means a loud passage laps over the words, which reads as text inside the light rather than beneath it.
The stages snapped because of what changes at each boundary, not because the timing was too fast. The crest swaps its whole sample array — live trace to take envelope — and an array cannot be interpolated, so no animation curve was ever going to help. Same for the caption: SwiftUI kept one view across transcript → processing → result and swapped its contents, so the text popped while the bloom jumped to its new size. Both are now identity changes. The crest carries a phase (live / replay / settled) as its `.id`, and the caption carries its kind, so each boundary is an insert plus a remove — the two overlap and dissolve instead of one mutating in place. Three smaller things fell out of that: The sweep is interpolated between task pushes. It arrives at ~12Hz, which is the difference between a travelling light and a row of jumps. The one-frame gap at the end of a take is closed. `takeEnvelope` was filled by the processing task a frame after the state flipped, so the crest blinked out and back on every single take; the fold now runs inline for that frame. Timing is asymmetric. Arriving is feedback that a key press worked and should feel immediate (0.24s); leaving is not urgent and a quick cut reads as a glitch (0.55s). Captions leave by fading only — shrinking back up while a replacement grows down looked like two things fighting for the same spot.
…ng at The ambient panel is the only window in the app sized to an entire display, and it was positioned exactly once — at show(). NotchRecorderPanel has observed didChangeScreenParameters since it was written; this one never did. So anything that moved the active screen out from under it left the light stranded: still drawing, on a display the user was no longer looking at, or at a size that display no longer had. From the front that looks like the UI vanishing mid-take while recording carries on, because that is exactly what happened. Four causes, needing different signals, so the net is deliberately wide: a display connected or disconnected, a resolution or scaling change, focus moving to another display, and the space changing to or from a full-screen app. On top of those there is a 1s watchdog — this window failing silently is worse than two rect comparisons a second, and I would rather not find a fifth cause the same way I found these. Also in this change: **Screen capture.** The panel sets sharingType = .none, so the light is absent from screen recordings and shared screens. This is the actual fix for the share-safety concern and it needs no setting; a light around the whole display means nothing to a viewer and cannot be removed after the fact. **Per-app opt-out**, for the cases capture exclusion does not cover — a video call where the light frames your face, a colour-critical editor, a presentation. Recording is unaffected; only the light goes away. **A light-background scheme.** The dark one works by adding light: pale colours, wide bloom, white text. Added light only reads against something darker than itself, so on a white document the whole effect disappears, and turning the opacity up only makes haze. The light scheme inverts the physics rather than the palette — deep saturated colours, tighter bloom, stronger rim, near-black text. The light stops behaving like a glow and starts behaving like ink soaking into the edge of the page. It follows the app's appearance setting rather than reading the pixels underneath. Sampling the real background means capturing the screen continuously, which needs Screen Recording permission — a wildly disproportionate ask for a colour choice. The setting is the honest proxy and is directly overridable when it is wrong. **Reduce Motion** now flattens the crest to a static band and stops the sweep travelling. A light rippling across the whole display is precisely the motion that setting exists to switch off, and both still communicate without animating. **The arbiter is extracted and tested.** State, crest phase and caption priority were inline in the view and untestable, and both of this session's silent failures lived there. AmbientPresentation is a pure function now, with 19 tests covering the priority order and the two cases that broke. **Transitions are plain cross-dissolves**, quick in both directions. Scale and offset were doing too much at this size. **One performance change**: bloomWidth is quantised to 3pt steps. It drives a blurred stroke the size of the display — the most expensive thing drawn here — and at 30Hz it was re-rendering for changes far below the threshold of sight.
The first light scheme did not work, and the reason is worth keeping. It changed the colours — deeper, more saturated — and left the compositing alone. Every layer here is drawn with ordinary source-over alpha, and that is the whole problem: colour over black *is* the colour, so it reads as light; colour over white washes toward white at any alpha below 1, however saturated the source. Raising the alpha does not rescue it either, it just walks the band from invisible haze to an opaque coloured rectangle, which is the exact thing this design exists to avoid. A glow cannot be added to something already at full brightness. So the light scheme no longer tries to add light. It lays a soft dark vignette in the same shape first, and puts the colour on top. The colour now has something darker than itself to be light against — the condition the dark scheme gets for free from the screen. Locally the display edge becomes a dark surround with a lit edge inside it: the same instrument, working the same way, on ground that could not otherwise carry it. The vignette is drawn once, above paint() rather than inside it. paint() runs twice while replaying — dim pass, then clipped lit pass — and stacking the darkness would have made the finished half of the take visibly murkier than the half still waiting. The rim also does far more work on white, where a bloom means almost nothing and a defined contour means everything, so the hairline roughly doubles and the blur tightens further. Because the appearance setting is only a proxy for what is actually on screen, there is now an explicit override: Match appearance / Tuned for dark / Tuned for light. A dark-mode system full of white documents is exactly the case the proxy gets wrong. Indeterminate sweep drops to 0.5s.
…tead of glow **Render scope.** Profiling two live takes put 16.8% of one core in the SwiftUI render path, and only 3.5% of that was drawing. The other 12.2% sat in AG::Graph::UpdateStack::update — SwiftUI walking its graph, 30 times a second, to rediscover that only the waveform had moved. Deciding what to draw cost nearly four times as much as drawing it. The cause was scope, not cost. trace, level and elapsed were @State on AmbientRecorderView, whose body builds the entire surface — frame, crest, notch halo, caption, mode strip, take bar — so every audio sample invalidated all of it. They now live on an @observable AmbientMeter, read only by the leaf that draws them: AmbientFrameLayer reads level, AmbientCrestLayer reads trace, AmbientTakeClock reads elapsed. Because Observation tracks reads per view, the parent reads none of them and stops re-evaluating at audio rate. Three things had to move with them or the split would have leaked: * hasTake is a Bool that flips once, not takeSamples.count — reading a count in the parent would have reintroduced a 10Hz whole-surface invalidation through the back door. * bloomWidth and notchHalo derive from level, so they moved into the frame layer with it. * The take bar takes its clock as a @ViewBuilder slot. As a stored TimeInterval, the 10Hz tick invalidated the enhancement label and the cancel button alongside it. Geometry is now resolved once into AmbientGeometry and shared, rather than each view asking NSScreen independently — a 1pt disagreement between the crest's baseline and the notch halo shows up immediately as a seam. It re-resolves on screen-parameter changes and at the start of each take. **Light backgrounds, second attempt.** The vignette was the right instinct wrongly applied: the colour does need something darker than itself, but spread across the whole band at a 22pt blur it reads as a grey smudge over the top of the screen. So the light scheme stops trying to be a glow and becomes ink. The band narrows to 62%, the blur drops to 40%, the colour goes to near-full strength against the bezel, and the contour — nearly decorative on black — becomes the main event at 2.4x width and 0.95 alpha. The only darkness left is a tight shadow a couple of points under the contour, which lifts the edge off the page without hazing it. Every alpha the crest uses is now a named palette value rather than a literal, because the two schemes disagree about all of them and burying that in the drawing code is how the first attempt went wrong.
…mes to a number The light scheme failed twice for the same reason, and I only found it by measuring. Both times I chose colours by how vivid they looked. On white the thing that matters is how *dark* they are, and those two pull in opposite directions — a vivid orange is a bright orange, and brightness is what a white page already has. Measured against their own ground: dark mint 12.28:1 amber 12.03:1 salmon 7.23:1 (avg 10.5) light green 3.04:1 orange 2.48:1 red 4.82:1 (avg 3.4) The amber at 2.48:1 was close to invisible, which is exactly what you were looking at. The light set is now deep ink rather than bright pigment — forest 6.93:1, bronze 5.53:1, oxblood 7.47:1. Less lively in isolation, far more legible in place, and on a white page a restrained dark edge reads as considered where a vivid one reads as an error. With deep ink underneath, the surrounding treatment no longer has to shout, so the contour shadow drops 0.34 → 0.22 and the outer halo 0.18 → 0.14. The caption scrim also stops being pure white: over a white page a white scrim is invisible, and the caption needs a pool it is visibly sitting in rather than text floating on the document. AmbientPaletteTests locks both schemes above 4.5:1 against their own ground, so this cannot regress by eye again. It also asserts the inversion itself — every light colour darker than its dark counterpart — which is what fails if someone later brightens the light scheme to make it livelier. One test in that file was wrong on the first pass and is worth recording. It checked that the three states were tellable apart by comparing their luminance, and failed on the *dark* palette, which has been fine all session: mint and amber sit at a 1.02:1 luminance ratio while being obviously different colours. Luminance ratio answers "can I read this against that", not "are these two different colours". It now measures CIE Lab distance, where the closest pair is bronze against oxblood at ~38 dE against a floor of 25. dE says nothing about colour-vision deficiency, and green/amber/red is precisely the axis that fails there. The mitigation is not in the palette: no state is carried by colour alone. A problem always writes the reason, processing always sweeps, listening always modulates.
…o darken The forest green looked murky on white, and there is a reason it was always going to. Green carries 71% of perceived luminance. A green dark enough to read against white has therefore had its green channel crushed, and what survives is a dark slate — the hue is gone. Measured as Lab chroma, the forest sat at 31 while cobalt at the same lightness reaches 69. Blue contributes only 7% of luminance, so it can be deep and still be intensely blue. It is the one hue that survives being darkened, which makes it the right choice here and green the wrong one. Cobalt is also better on the numbers that already mattered: 7.95:1 against white versus the forest's 6.93:1. Choosing blue for listening surfaced a second problem worth fixing while here. Green/amber/red is the axis that collapses under red-green colour blindness, and the light scheme had walked into it: a deep bronze against a pure oxblood measured 11.6 dE for a deuteranope, which is the same colour in practice. The problem colour now carries a blue lean — wine rather than oxblood — taking that pair to 26.6 while staying unmistakably an alarm. Final light set, all measured: listening cobalt 7.95:1 chroma 69 working bronze 5.27:1 chroma 57 problem wine 6.93:1 chroma 65 separation normal deuteranope listening/work 116.6 128.4 listening/prob 89.5 102.9 work/prob 52.0 26.6 The dark scheme is untouched: it was working, and its weakest colour-blind pair (amber against salmon, 22.8) already clears the floor. AmbientPaletteTests now also simulates deuteranopia and holds every pair above 20 dE, so the trap that caught the light scheme cannot catch it again. Redundancy elsewhere is still the real defence — a problem always writes the reason, processing always sweeps — but colour should not be actively misleading.
… app's theme You had green on a white page, and the cause was a design error rather than the proxy being imprecise. `auto` resolved through @Environment(\.colorScheme), which inside the panel is the *app's* appearance. Your settings: AppAppearancePreference = dark, macOS system theme = Light. Wanting dark chrome for VoiceInk's own windows on a Light Mode Mac is completely normal, and it says nothing whatsoever about the white document the light is drawn over. So the palette confidently picked the dark scheme and put mint green on white. I had called this a proxy limitation; it was closer to reading the wrong variable. It now measures. The app already ships ScreenCaptureKit for context capture, so where that permission exists there is no reason to guess: capture the display at 64x40 once per take, average the relative luminance of the top band, and decide from that. Four constraints shape it: * It never *requests* permission — CGPreflightScreenCaptureAccess only. Prompting for Screen Recording in order to choose a colour would be a wildly disproportionate trade. * The fallback is the **system** appearance, not the app's. That alone fixes the case above, with or without the grant. * It samples the top band rather than the whole screen. The light lives against the edges, and a dark wallpaper behind a white document would otherwise outvote the thing actually being looked at. * Hysteresis of 0.08 around the threshold, so a mid-grey window cannot make the palette flicker between schemes. 64x40 is deliberate: this is a luminance average, so resolution buys nothing and costs real time. That is ~2,500 pixels, once per take, off the main path. The setting is now "Match background" rather than "Match appearance", which is what it finally does. The explicit overrides stay for when the permission is absent or the measurement is simply not what you want.
Screen-wide luminance was measuring the wrong surface. It averages wallpaper, menu bar and every other window, so a dark editor on a bright desktop still came out light. What matters is the thing being worked in. macOS does not expose another process's NSAppearance, so a window's theme cannot be asked for — but it can be seen, because a dark-themed window renders dark pixels. So the sensor now finds the frontmost app's largest on-screen window, captures that window alone at 64x40, and averages it. Largest rather than first: apps carry palettes, inspectors and toolbars that are on screen but are not what you are looking at. Falls back in order — the whole display when there is no usable window (dictating at the desktop), then the system appearance when Screen Recording permission is absent. VoiceInk's own windows are skipped throughout; measuring our own settings window would answer a question nobody asked. The setting reads "Match the app I'm dictating into", which is now literally what it does.
Switching the recorder style to Mini left the ambient window alive, and the watchdog I added to survive display changes turned that from harmless into serious: rebuildVisiblePanel bailed out early when the panel was not on screen, so destroyWindow was never called, isShowing stayed true, and the 1s watchdog kept calling orderFrontRegardless. A display-sized window was forcing itself to the front once a second, forever, with the recorder style set to Mini. That is why the cursor stopped working and Settings appeared to lose its options — the dropdown was opening underneath an invisible full-screen window. Two fixes, because one of them should have been there from the start: The teardown now runs whether or not the panel was visible. A hidden window is not an inert one once it owns a repeating task. And the panel now ignores mouse events by default, switching them on only for the three moments this surface has anything to click: the result peek's buttons, the silence countdown's reprieve, and the mode strip and cancel during a take. The hosting view still passes through everything it does not draw, but that is a second line of defence now rather than the only one. A display-sized window that accepts events is one hit-testing surprise away from swallowing every click on the machine, and the cost of being wrong is the user losing their mouse.
…saction Destroying the panel synchronously crashes. GraphHost can already have an async transaction queued, and when it lands it calls NSHostingView.setNeedsUpdate → setNeedsUpdateConstraints → _postWindowNeedsUpdateConstraints, which throws on a window that is going away: -[NSWindow(NSDisplayCycle) _postWindowNeedsUpdateConstraints] -[NSView setNeedsUpdateConstraints:] SwiftUI NSHostingView.setNeedsUpdate() SwiftUI NSHostingView.beginTransaction() SwiftUI GraphHost.asyncTransaction(...) The race is not new — it is behind two EXC_BREAKPOINT reports from earlier today, both of which I had looked at and set aside as unreproducible AppKit layout exceptions with no frames of ours. Tearing the panel down on a style change is what turned it from rare into reliable, and that is what finally gave it a backtrace. The window is now ordered out immediately and released on the next turn of the run loop with its SwiftUI content detached, so anything already queued drains against a window that is still valid.
When this picks the wrong scheme there is currently no way to tell from the outside whether it measured something and disagreed with you, or never measured at all and quietly fell back to the system theme. Those need opposite fixes, and guessing between them has already cost several rounds. So it says which: the permission state when there is none, the measured luminance against the threshold when there is, and the capture error when one happens. Not behind a debug flag — it fires once per take, and a silent fallback is exactly the failure this is meant to make visible.
The unified log turned out to be useless here — `log show` returns nothing for this process at all, at any level, including the launch line that fires every start. So the diagnostic I added last commit could never have been read. The reading now goes where both of us can see it: persisted, and shown under the Ambient Light picker. "Last take: window brightness 0.84 — using light colours" "Screen Recording is off, so the window can't be read — following the system theme (light)" "Couldn't read the window (…) — kept the last choice" "Not measured yet — record once to see" Persisted rather than held in memory because the ambient panel is built and destroyed around takes while Settings is a different view entirely, so there is no live object for it to read. This is worth keeping past the debugging. "Why did it choose these colours" is a question a user can now answer, and the three failure modes — measured and disagreed, never measured, capture failed — need opposite responses and were otherwise indistinguishable from the outside.
…ce the stale grant Two problems, both traceable to the same recorded reading: "Screen Recording is off, so the window can't be read — following the system theme (light)" **Dark backgrounds stopped getting mint** because CGPreflightScreenCaptureAccess returns false even though the toggle in System Settings reads as on. Screen Recording is granted per binary, not per designated requirement, so it goes stale on every rebuild: the pane keeps showing the old grant while the new build is not actually allowed. With no measurement the sensor fell back to the system theme, which is Light here, so every take got the light scheme regardless of what was on screen. Nothing in the app can repair that grant, so Settings now says what happened and offers a button straight to the Screen Recording pane. Removing and re-adding VoiceInk there is the fix. **The cobalt crest was lifeless**, and that one is a real design mistake rather than a bug. The deep ink set is right for the frame and the rim, where the job is legibility against white. It is wrong for the waveform, which has a different job: a voice should look alive, and a dark blue at 7:1 contrast just sits there. Mint works on black precisely because it is bright, and the light scheme threw that away when everything went deep. The crest is now two-tone on light grounds — a vivid core inside a deep contour. The rim still carries legibility, since it is the measured colour and unchanged, which frees the fill to be luminous because something that is not luminous is bounding it. The crest also keeps more of its bloom than the frame does (0.8 vs 0.4 blur, halo 0.30 vs 0.14); tightening it to match the frame is what flattened it in the first place.
…tem theme With the Screen Recording grant stale there is no measurement, and the fallback was the macOS theme. That is consistently wrong for anyone whose Mac is in Light Mode but who works in dark apps — every take got the light scheme regardless of what was on screen, which is why dark backgrounds were coming out cobalt instead of mint. VoiceInk's own Appearance setting is the better guess. It is not a statement about any particular window, which is exactly why it must never outrank a real measurement — reading it as the *primary* signal is what put mint green on a white page several commits ago. But someone who has explicitly set this app to Dark has said something about the world they work in, and as a fallback that beats asking the OS. `System` means they have not said, so the OS still answers. Order is now: the measured window, then the app's explicit Appearance, then the system theme. Only the first is about the window actually behind the light; the other two are guesses, in descending order of how much the user has told us.
"Tuned for dark backgrounds" was ambiguous in the worst way. It reads as *when* the light appears rather than *how* it is drawn, so the natural guess is that it hides the light on light backgrounds. It never did — every option always shows the light, and they differ only in which palette it uses. Named by their output instead: Match the app I'm dictating into Always bright — mint, amber, red Always deep — cobalt, bronze, wine Each now carries a line saying what it does and, more usefully, where it fails: bright colours fade out against a white page, deep ones look heavy on a dark app. That is the whole reason there is a choice here, and it was the one thing the picker never said.
19 tests over the two pieces that had none, and where the worst bug of the
session lived: a display-sized window that ended up forcing itself to the front
and swallowing every click on the machine.
AmbientWindowManager needed one seam to be testable at all — it built its hosted
view inline from the engine, so nothing about its lifecycle could be exercised
without standing up the whole app. The view is now injected, with the
engine-taking initialiser kept as a convenience.
What is asserted, roughly in order of how much it would hurt to get wrong:
* the window ignores the mouse by default, accepts it only while something is
clickable, and gives it back on hide. A hidden window holding the mouse is
not recoverable from the UI.
* hide and destroy both stick *past the watchdog's next tick*. The watchdog
exists to recover a window stranded by a display change and must never
confuse "hidden on purpose" with "stranded" — the two 1.4s tests are the
only slow ones here and they are the point of the file.
* the panel never becomes key, which is what broke Undo earlier in the session.
* the hosting view returns nil for a hit it does not draw, which is the whole
click-through contract.
* destroy before show, and show after destroy, are both safe. Switching
recorder style away and back does exactly that.
A gap worth naming: these lock the window's own contract, not the call site that
violated it. The teardown being skipped lived in RecorderUIManager, which cannot
be constructed in a test without the engine. That ordering is still verified by
reading rather than by a test.
…ther you stopped talking Silence auto-stop never fired, and the reason is a conflation in my own code. It was gated on `health == .silent`, which requires *every* average in a three-second window to sit under 0.06. That state answers "is this microphone dead". A fan, a keystroke, or a voice two rooms away is enough to keep a real room out of it indefinitely, so the trigger never armed. "Has this person stopped speaking" is a different question and wants a different signal. Peaks, not averages: speech is loud in the peaks even when the average is low — a whisper still reaches 0.41 — while room tone stays far below. The new `isQuiet` is true when nothing in the window peaked above 0.15, and auto-stop reads that instead. Read as a maximum rather than a proportion, so one word anywhere in the window holds the take open. Pausing between sentences must not start a countdown. Also resets the watch at the start of a take in ambient, which was missing: a silence carried over from a previous take could have stopped a new one on the spot. Ten tests, including the case that was failing in the field — a room with a fan in it, where averages sit above the silent threshold and nobody is speaking.
The dashboard counted words, minutes and sessions. Those go up, and knowing they
went up changes nothing. Every row of this card exists because there is
something to do about it.
Speaking pace — turns the time-saved claim from an assertion into arithmetic
you can check, and answers whether dictation is faster *for you* rather than
faster in general.
Typical take — median, so one abandoned recording does not redefine typical.
Calls out a longest take worth splitting when there is one.
Typical wait — split into transcription and enhancement, because those are the
only two levers and they need opposite fixes. If enhancement is 60% of the
wait, a faster transcription model will not help, and nothing in the app was
saying so.
Busiest mode — weighted by words, not takes. A mode carrying 90% of the work
means the others are costing a decision every take and returning nothing.
Days used — the only honest read on whether this is a habit or an experiment.
All of it comes from SessionMetric, which has been recording every one of these
figures all along and surfacing none of them.
The computation is pure and tested — 21 tests — because the arithmetic has traps
in it. Pace is words over total seconds rather than a mean of per-take rates, or
a two-word correction would weigh the same as a five-minute dictation. Take
length is a median for the same reason. Mode share is weighted by words. And
nothing renders below five takes: a confident number computed from three is
worse than no number, and it teaches people to distrust the rest of the screen.
…ionary hits
Three signals the app produced and immediately discarded. All three are optional
or defaulted on SessionMetric, so SwiftData migrates in place and existing rows
simply carry nils.
targetBundleIdentifier — captured at record time rather than at delivery,
which is safe because every recorder panel is non-activating: whatever is
frontmost when the metric is written is the app about to receive the paste.
wasUndone — set afterwards, when a result is taken back. Taking a result back
is the strongest signal this app ever gets that a take went wrong, and until
now it left no trace whatsoever. The peek now carries the transcription id so
the undo can be attributed to the right session; marking is best-effort,
because a failed or canceled take never has a metric to mark.
dictionaryHitCount — how many replacements fired. Nil rather than zero for
takes that predate this, because "we didn't measure" and "nothing fired" are
different facts and the dashboard has to be able to tell them apart.
None of these can show anything for existing history. They start accumulating
now, which is exactly the trade-off flagged before building them.
Six summaries, all pure and tested, plus the fetch that feeds them.
Transcripts are now loaded alongside session metrics, because the two tables
answer different questions and only one of them can answer some. A SessionMetric
is written only when a take completes, so measuring failure from that table
would report a flawless record forever. Failure, cancellation, what enhancement
actually changed, prompt usage and retained audio all come from Transcription.
Reliability — success rate excluding cancellations from the denominator.
Abandoning a take is a decision, not a malfunction, and counting it as one
would make a careful user look like they have broken software. Cancellation
gets its own figure instead.
Enhancement impact — how much of your text the AI pass actually rewrites,
measured as characters that did not survive. Answers whether enhancement is
earning its wait. Longest-common-subsequence rather than full edit distance:
same answer to this question, two rows of memory instead of a full table,
and it runs over every take in the window.
Re-dictation — takes followed by another within 30 seconds. The closest thing
to an accuracy measure available without asking the user anything: nobody
records "that was wrong", they just say it again, and the timestamps already
know.
Destinations, dictionary, model leaderboard — from the fields added last
commit. The leaderboard puts speed and outcome side by side because either
alone misleads: the fastest model is a poor choice if a chunk of its takes
get undone.
Two distinctions the tests pin down, because both are easy to get wrong and
silently misreport: a nil dictionary count means "not measured yet" and must
never be folded into "nothing fired", and a take with no destination is a gap in
the record rather than an app.
Audio size is stat-ed per file rather than stored at record time. More
expensive, but a stored size goes stale the moment a file is cleaned up behind
the app's back.
Six figures on one grid, every one of them a link. The overview answers "how am I doing"; the detail answers "why is that the number", which is the only question a summary figure can provoke and the one a scoreboard never lets you ask. Drill-in rather than tabs, because the overview is itself the most useful screen: most visits should end there, having answered the question at a glance. Tabs would have made the summary one destination among several and buried the part that gets read. Every card carries a figure *and* a sentence. A number alone is a scoreboard entry — the line under it is the reason the card is on screen, and a card that cannot produce one has no business being there. Some of those sentences change with the data rather than restating it: when enhancement is most of your wait it says a faster transcription model will not help, and when the AI pass is rewriting under 2% it says outright that it is running for nothing. Cards with no data yet stay visible and greyed, saying what they are waiting for. Hiding them would make the dashboard change shape as data arrives, which is disorienting and makes it look broken on first run. Charts are used only where a distribution is the point — the split of the wait, and model speed side by side. Bundle identifiers are resolved to real app names and icons; nobody should be shown "com.tinyspeck.slackmacgap". The single stacked card added earlier is superseded by this and removed.
…rrency
Upstream landed a license/keychain resilience change while this branch was in
flight. Fourteen files conflicted, all along the same seam: upstream reworked the
license layer's behaviour, this branch had reworked the same files' concurrency
and observation.
Resolved by keeping upstream's semantics wholesale and re-expressing them in this
branch's idiom, never the other way round:
* LicenseViewModel keeps upstream's `shared` singleton and its tightened
`private(set)` access, but stays @observable rather than reverting to
ObservableObject. Reverting one type would have left the app split between
two observation systems for no reason.
* LicenseStoring, PolarServicing and the new storage value types are taken as
upstream wrote them. Both protocols gain `Sendable`, which they need because
a main-actor view model awaits them — without it strict concurrency has to
assume an arbitrary conformer is unsafe to hand across the boundary.
* Views take upstream's new `licenseKeyDraft` bindings and star-prompt
coordinator. GitHubStarPromptCoordinator stays ObservableObject: it is
upstream's new code, this PR has no business migrating it, and mixing the
two systems is legal.
Three upstream additions needed adjusting to compile under strict concurrency,
which is switched on by this branch and was not by theirs:
* GitHubCLIStarService cached its resolved `gh` path in mutable statics. Two
concurrent callers would race, so the cache moved into an actor. The search
itself still runs off it.
* LicenseViewModel's deinit cancels two Tasks, and deinit is nonisolated under
Swift 6. Marked nonisolated(unsafe) — `Task.cancel()` is documented safe from
any thread, so this is one of the few places that escape hatch is the answer
rather than a shortcut.
* LicenseKeychainAccessibilityMigration holds a UserDefaults, which is
thread-safe but cannot say so. Same @unchecked Sendable bridge already used
for KeychainService.
155 tests pass and the app launches clean.
Written for a reviewer who has not seen any of this. Leads with what each part bought rather than what it changed, since the diff already says what changed. Includes a section arguing against the PR's own size, and four other things I would raise if I were reviewing it. A reviewer finding those themselves costs them time; a contributor who has already found them costs nothing.
DhavalFatnani
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
40 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="VoiceInk/Services/AudioClipExporter.swift">
<violation number="1" location="VoiceInk/Services/AudioClipExporter.swift:32">
P2: Exporting a clip can silently overwrite an existing same-basename `.txt` file, because the save panel only confirms the WAV replacement and this write replaces the sidecar unconditionally. Check for an existing sidecar and prompt or choose a unique name before writing it.</violation>
<violation number="2" location="VoiceInk/Services/AudioClipExporter.swift:81">
P1: Exporting over the source recording can destroy it: the writer opens `destination` directly even when it is `source`, truncating the input before the copy completes. Reject identical source/destination URLs (and preferably export to a temporary file before replacing a destination).</violation>
</file>
<file name="VoiceInk/Transcription/Engine/VoiceInkEngine+Protocols.swift">
<violation number="1" location="VoiceInk/Transcription/Engine/VoiceInkEngine+Protocols.swift:38">
P1: A retry finishing after another take can undo the newer take and paste stale text. Confirm the peek's transcription is still latest before issuing Cmd+Z.</violation>
</file>
<file name="VoiceInk/Transcription/Engine/RecorderUIManager.swift">
<violation number="1" location="VoiceInk/Transcription/Engine/RecorderUIManager.swift:90">
P1: Result peek is shown while `isRecorderPanelVisible` remains false, so recorder shortcuts treat the visible peek as dismissed and can start a new take. Set visibility through the state property so observers and shortcut routing stay consistent.</violation>
</file>
<file name="VoiceInk/Views/Dashboard/DictationInsightFacts.swift">
<violation number="1" location="VoiceInk/Views/Dashboard/DictationInsightFacts.swift:108">
P1: Failed AI passes are counted as enhanced rewrites because their persisted error message is nonempty `enhancedText`, so repeated provider failures can make the dashboard report a large rewrite rate. Carry successful-enhancement state/duration into `TranscriptFact` and require it here before incrementing enhancement metrics.</violation>
</file>
<file name="VoiceInk/Views/Dashboard/DashboardStatsModels.swift">
<violation number="1" location="VoiceInk/Views/Dashboard/DashboardStatsModels.swift:280">
P1: Custom agent: **Backward compatibility**
Adding a new non-optional `Codable` property to `DashboardStatsSummary` breaks loading of previously persisted dashboard snapshots. The struct is encoded to JSON via `DashboardStatsSnapshotStore`, and synthesized Swift `Decodable` does not respect default property values — older JSON files missing the `insights` key will throw `keyNotFound` on decode and cause the cache to return `nil`. This forces a full recalculation for existing users after upgrade.</violation>
</file>
<file name="VoiceInk/Modes/ModeFormWarmupStore.swift">
<violation number="1" location="VoiceInk/Modes/ModeFormWarmupStore.swift:121">
P2: Custom agent: **Backward compatibility**
`deinit` does not cancel `snapshotObserver`, so the underlying `ObservationBridge` observation remains registered with the observed `@Observable` services after the store deallocates. This is a regression from the previous Combine-based implementation, where `AnyCancellable` subscriptions were automatically torn down on dealloc. Every other class using `ObservationBridge` in this codebase calls `cancel()` in `deinit` (e.g., `AIService` and `RecorderPanelShortcutManager`). Adding `snapshotObserver?.cancel()` to `deinit` restores consistent lifecycle cleanup.</violation>
<violation number="2" location="VoiceInk/Modes/ModeFormWarmupStore.swift:123">
P1: Custom agent: **Backward compatibility**
`configure()` no longer rebuilds the snapshot synchronously when dependencies change, so the UI can display stale data after reconfiguration. In the old code, `refreshSnapshot()` ran immediately whenever `dependenciesChanged` was true; now it is only reached through `ObservationBridge`, whose initializer schedules the refresh via an async `Task` rather than calling it synchronously. Add an explicit `refreshSnapshot()` call inside the `dependenciesChanged` branch.</violation>
</file>
<file name="VoiceInk/Views/History/InlineHistoryView.swift">
<violation number="1" location="VoiceInk/Views/History/InlineHistoryView.swift:108">
P2: Tags outside loaded pages receive no filter chip, preventing users from filtering by them until they scroll to the relevant page. Source filter tags from an unfiltered tag query/list instead of paged rows.</violation>
<violation number="2" location="VoiceInk/Views/History/InlineHistoryView.swift:351">
P1: Custom agent: **Backward compatibility**
The new "Select All" button in the selection bar at `InlineHistoryView` selects only the current page of non-pinned transcriptions and breaks the toggle behavior when pinned rows exist. The previous `selectAllTranscriptions()` fetched every matching transcription across all pages; the replacement only assigns `displayedTranscriptions` to `selection`. Because `allSelected` checks against `loadedTranscriptions` (`pinnedTranscriptions + displayedTranscriptions`), clicking "Select All" can never reach the `allSelected == true` state when pinned items are present, so the button never toggles to "Deselect All". Additionally, results beyond the first page are no longer selected, regressing the prior select-all-across-results functionality.</violation>
</file>
<file name="VoiceInk/Views/Recorder/AmbientPresentation.swift">
<violation number="1" location="VoiceInk/Views/Recorder/AmbientPresentation.swift:111">
P1: A new take started during the four-second peek window shows the prior take’s result caption instead of the active take’s countdown/problem/live text. Gate the result slot to `.idle`, matching `resolveState`’s stale-peek handling.</violation>
</file>
<file name="VoiceInk/Services/OllamaService.swift">
<violation number="1" location="VoiceInk/Services/OllamaService.swift:6">
P1: Changing the server URL or model during an enhancement can race its request construction, because `@unchecked Sendable` declares this mutable observable object safe across executors without synchronization. Isolate the service to `@MainActor` (its sole owner already is) or protect all mutable state before retaining Sendable.</violation>
</file>
<file name="VoiceInk/Views/Recorder/NotchRecorderPanel.swift">
<violation number="1" location="VoiceInk/Views/Recorder/NotchRecorderPanel.swift:12">
P1: NotchRecorderPanel still overrides canBecomeKey/canBecomeMain to return true, so the new needsKeyFocus mechanism never applies to it: clicking a peek button on the notch panel still pulls key focus off the app, which is exactly the bug this change is meant to fix. Remove these two overrides so NotchRecorderPanel inherits KeyablePanel's needsKeyFocus-based behavior, like MiniRecorderPanel does.</violation>
</file>
<file name="VoiceInk/Views/Recorder/AmbientRecorderView.swift">
<violation number="1" location="VoiceInk/Views/Recorder/AmbientRecorderView.swift:190">
P1: Clicking Ambient’s Cancel ends the recording but leaves `isRecorderPanelVisible` true and its fullscreen ambient window alive; because idle Ambient draws no controls, the next recorder shortcut only dismisses this invisible panel instead of starting a take. Route this action through the UI manager’s cancelling/dismissal path, or dismiss the panel after cancellation.</violation>
</file>
<file name="VoiceInk/Views/Recorder/MiniRecorderView.swift">
<violation number="1" location="VoiceInk/Views/Recorder/MiniRecorderView.swift:176">
P1: Quiet familiar takes can auto-stop without showing the countdown or “Keep recording” control because the density gate suppresses the strip; let an active countdown bypass that gate.</violation>
</file>
<file name="VoiceInk/Transcription/Streaming/XAIStreamingProvider.swift">
<violation number="1" location="VoiceInk/Transcription/Streaming/XAIStreamingProvider.swift:5">
P1: Custom agent: **Flag Security Vulnerabilities**
The `@unchecked Sendable` conformance suppresses the compiler's concurrency safety checks, but the mutable state (`eventsContinuation`, `forwardingTask`) is not actually synchronized. The comment claims state is "guarded by locks or confined to one task," yet there are no locks, actors, or serial queues in the file. Since the protocol now requires `Sendable` and instances are explicitly passed across isolation domains, concurrent access to these properties can produce data races and crashes. Consider adding an `NSLock` (or a serial actor if appropriate) to guard all mutable state, or remove `@unchecked` and refactor to use only `Sendable`-safe types.</violation>
</file>
<file name="VoiceInk/Shortcuts/ShortcutStore.swift">
<violation number="1" location="VoiceInk/Shortcuts/ShortcutStore.swift:23">
P1: Custom agent: **Backward compatibility**
Adding `@MainActor` to the existing `ShortcutStore` methods (`setShortcut`, `seedShortcut`, `removeShortcutStorage`) changes their actor isolation contract and can break compilation for non-isolated callers. I found several existing callers that are not `@MainActor`-isolated, such as `ShortcutMigration.shortcutSelection` and `ShortcutMigration.migrateLegacyKeyboardShortcut` in `ShortcutMigration.swift`, and `ModeConfig.removeConfiguration` in `ModeConfig.swift`. Under Swift strict concurrency, synchronous calls to MainActor-isolated methods from non-isolated contexts will produce compilation errors. If main-thread safety is needed, consider wrapping the `UserDefaults` and `NotificationCenter` operations internally (e.g., with `MainActor.run`) rather than shifting the isolation requirement onto the method signature itself, so existing off-main-actor callers continue to compile.</violation>
</file>
<file name="VoiceInk/Transcription/Whisper/WhisperModelProvider.swift">
<violation number="1" location="VoiceInk/Transcription/Whisper/WhisperModelProvider.swift:7">
P1: Custom agent: **Backward compatibility**
Adding `Sendable` to the `@MainActor` protocol `WhisperModelProvider` introduces a new compile‑time requirement that the existing conformer `WhisperModelManager` does not satisfy. The class holds non‑Sendable stored state such as `WhisperContext?` and callback closures, so under Swift 6 the existing `extension WhisperModelManager: WhisperModelProvider {}` will no longer compile. This breaks backward compatibility and prevents the project from building. Either remove `Sendable` from the protocol until all conformers are updated, or update `WhisperModelManager` (and any mocks/stubs) to satisfy `Sendable` in the same changeset.</violation>
</file>
<file name="Makefile">
<violation number="1" location="Makefile:66">
P2: `make test` cannot run on Intel Macs because its only destination requires `arm64`; let Xcode select the host macOS architecture instead.</violation>
<violation number="2" location="Makefile:68">
P1: Custom agent: **Backward compatibility**
The `local` target no longer forces ad-hoc signing during the build, which breaks `make local` on machines that haven't created the stable self-signed certificate yet.
The old Makefile passed `CODE_SIGN_IDENTITY="-"` on the xcodebuild command line. Command-line build settings have the highest priority in Xcode and override the target-level `"CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development"` defined in the project. Removing that flag means xcodebuild now falls back to `"Apple Development"`, an identity most local developers don't have. Because `CODE_SIGNING_REQUIRED=NO`, the build succeeds but produces an *unsigned* app when no cert is found. On Apple Silicon, unsigned apps are blocked by Gatekeeper and won't launch without a manual security exception, breaking the documented local-build workflow.
`LocalBuild.xcconfig` does contain `CODE_SIGN_IDENTITY = -`, but project-level/xcconfig settings are outranked by target-level settings, so it has no effect here. Restoring the command-line override restores the previous ad-hoc fallback behavior.</violation>
</file>
<file name="VoiceInk/Transcription/Streaming/FluidAudioNemotronStreamingProvider.swift">
<violation number="1" location="VoiceInk/Transcription/Streaming/FluidAudioNemotronStreamingProvider.swift:8">
P1: Custom agent: **Flag Security Vulnerabilities**
`FluidAudioNemotronStreamingProvider` declares `@unchecked Sendable` but has unsynchronized mutable state. The added doc comment says state is "guarded by locks or confined to one task," yet there are no locks in the file, and `StreamingTranscriptionService` calls this provider from a `Task.detached` send loop while also mutating it on the main actor. This suppresses the compiler's data-race checks without an actual safety guarantee, creating a memory-safety risk. Add an `NSLock` (or use another synchronization mechanism) around accesses to `manager` and `eventsContinuation`, following the pattern already used by `FluidAudioStreamingProvider` and `StreamingMetrics` in the same module.</violation>
</file>
<file name="VoiceInk/Services/AudioDeviceManager.swift">
<violation number="1" location="VoiceInk/Services/AudioDeviceManager.swift:78">
P1: Custom agent: **Backward compatibility**
The `currentInputDeviceName` property claims to return the name of the input the next take will actually use, but its logic diverges from `getCurrentDevice()` in every mode. When `inputMode` is `.systemDefault`, switching modes does not clear `selectedDeviceID`, so the property can report a stale custom device while the recorder uses the system default. In `.custom` mode it skips the availability check that `getCurrentDevice()` performs, returning the name of a disconnected device. In `.prioritized` mode it ignores `prioritizedDevices` entirely. Because the Signal Strip shows this name to help users diagnose an unhealthy input, an incorrect name breaks the feature. You can resolve this by delegating to `getCurrentDevice()` and looking up the resolved device name:</violation>
</file>
<file name="VoiceInk/Services/AIEnhancement/AIService.swift">
<violation number="1" location="VoiceInk/Services/AIEnhancement/AIService.swift:60">
P1: Custom agent: **Backward compatibility**
`AIProvider.defaultModel` and `AIProvider.availableModels` are now annotated with `@MainActor`, but these computed properties are pure functions that simply return constant strings based on the enum case. Adding main-actor isolation breaks compilation for existing non-main-actor callers such as `ModeRuntimeConfiguration`, `ModeFormWarmupStore`, `VoiceInkEngine`, and `AssistantChatService`, which reference these properties from plain structs or background classes.
Because the properties do not access any main-actor-only state, the `@MainActor` annotation is unnecessary and introduces a breaking API change. Remove `@MainActor` from the `defaultModel` and `availableModels` computed properties on `AIProvider`, or update every non-main-actor call site to hop to the main actor.</violation>
<violation number="2" location="VoiceInk/Services/AIEnhancement/AIService.swift:247">
P1: Custom agent: **Backward compatibility**
The switch from `@Published` to `@Observable` removed the blanket invalidation that used to refresh views reading derived Ollama state. Previously, any change to `isOllamaRefreshing` sent `objectWillChange`, so views displaying `connectedProviders` or the Ollama model list would update after a background refresh. Now those computed properties depend on `externalStateRevision`, but `refreshOllamaAvailability()` never bumps it. That means views that don't also read `isOllamaRefreshing` will stay stale after the refresh completes. To restore the old behavior, call `markExternalStateChanged()` after the Ollama service finishes updating.</violation>
</file>
<file name="VoiceInk/Transcription/Engine/TranscriptionDelivery.swift">
<violation number="1" location="VoiceInk/Transcription/Engine/TranscriptionDelivery.swift:53">
P1: Custom agent: **Backward compatibility**
The result-peek feature stores the in-flight request in a single shared property `pendingPeekSource` before `await paste(...)` and reads it later in `paste()` via `peek(for:)` **after** `await actions.dismiss()`. Because `TranscriptionDelivery` runs on `@MainActor`, that `await` can yield the actor; a second `deliver()` call may then overwrite (or `defer`-clear) `pendingPeekSource` before the first call reads it, so the peek shows the wrong transcription’s original text / enhancement status or is silently skipped.
Instead of shared mutable state, pass the `request` through to `paste(_:output:request:actions:)` and let `peek(for:request:)` build the result directly from that parameter, then remove `pendingPeekSource` entirely.</violation>
</file>
<file name="VoiceInk/Views/Recorder/AmbientCaption.swift">
<violation number="1" location="VoiceInk/Views/Recorder/AmbientCaption.swift:77">
P2: Ambient result peek expires after four seconds even while pointer is over its text or controls, so users can lose Undo/Retry while reading or moving to a button. Thread `setResultPeekHovered` into this caption and call it from a hover handler, matching mini/notch peek behavior.</violation>
<violation number="2" location="VoiceInk/Views/Recorder/AmbientCaption.swift:121">
P1: Custom agent: **Backward compatibility**
`AmbientCaption` reuses its `@State` across different takes because the parent applies `.id(captionIdentity)` and `captionIdentity` is `1` for every `.result` case. If a user toggles "Show original" on one result and the next take finishes with `canShowOriginal == false`, the new result displays `originalText` with no way to switch back. Consider adding `.onChange(of: kind) { _, _ in showingOriginal = false }` on the view body so the toggle resets whenever a new caption arrives.</violation>
</file>
<file name="VoiceInk/Transcription/Whisper/WhisperTranscriptionService.swift">
<violation number="1" location="VoiceInk/Transcription/Whisper/WhisperTranscriptionService.swift:7">
P1: The @unchecked Sendable conformance is unsound: the added comment claims the mutable state is "guarded by locks or confined to one task," but `whisperContext` is a plain var mutated throughout the async `transcribe()` with no lock or serial actor, so two concurrent transcriptions can interleave writes and can call releaseResources() on a context another call is still using (crashes/corrupt output). The compiler check is only suppressed by @unchecked; either guard the state with a real lock/actor or drop the conformance and serialize access.</violation>
</file>
<file name="VoiceInk/Transcription/Streaming/DeepgramStreamingProvider.swift">
<violation number="1" location="VoiceInk/Transcription/Streaming/DeepgramStreamingProvider.swift:8">
P1: Cancelling during an in-flight audio send can concurrently access this provider from the detached sender, forwarding task, and disconnect path, but `@unchecked Sendable` suppresses the diagnostics rather than synchronizing them. Isolate provider state in an actor (and await cancellation/drain before disconnect), or protect every shared access with synchronization before asserting Sendable.</violation>
</file>
<file name="VoiceInk/Transcription/Streaming/FluidAudioUnifiedStreamingProvider.swift">
<violation number="1" location="VoiceInk/Transcription/Streaming/FluidAudioUnifiedStreamingProvider.swift:8">
P2: Cancelling while an audio chunk is in flight can race `sendAudioChunk`'s read of `manager` with `disconnect()` clearing it; `@unchecked Sendable` suppresses the diagnostic without making mutable provider state safe. Isolate this provider in an actor or synchronize every access to `manager` (and lifecycle state) before asserting `Sendable`.</violation>
</file>
<file name="VoiceInk/Views/Recorder/AmbientLayers.swift">
<violation number="1" location="VoiceInk/Views/Recorder/AmbientLayers.swift:27">
P2: Moving a take between displays can retain the previous display’s notch dimensions, so a notch halo/crest is drawn on a non-notched screen (or vice versa). Refresh `geometry` when active-screen changes cause the panel to move, or derive it from the panel’s current screen.</violation>
</file>
<file name="VoiceInk/Services/TranscriptionAutoCleanupService.swift">
<violation number="1" location="VoiceInk/Services/TranscriptionAutoCleanupService.swift:5">
P2: Manual and startup cleanup now run their database fetch/delete and file removal loops on the UI actor, so a large transcription history can freeze the settings/app UI. Keep only shared state and UI notifications main-isolated; create/use the cleanup context in a detached/background task.</violation>
</file>
<file name="VoiceInk/Views/Recorder/NotchRecorderView.swift">
<violation number="1" location="VoiceInk/Views/Recorder/NotchRecorderView.swift:139">
P2: Processing estimate reads a previous take's duration (or zero) when transcription starts, so the new progress row is absent or predicts the wrong wait. Capture current audio duration before entering `.transcribing`, then expose that value to this view.</violation>
</file>
<file name="VoiceInk/Shortcuts/ShortcutMigration.swift">
<violation number="1" location="VoiceInk/Shortcuts/ShortcutMigration.swift:32">
P1: Custom agent: **Backward compatibility**
Adding `@MainActor` to `ShortcutMigration` retroactively isolates all existing static members. `ShortcutStore` (a non-isolated enum) currently calls `ShortcutMigration.removeLegacyCustomRecordingShortcut` and `removeLegacyKeyboardShortcut` synchronously; once those methods inherit `@MainActor`, the calls will fail under Swift strict concurrency or Swift 6 because they lack `await` / `MainActor.run`. To preserve backward compatibility, either avoid `@MainActor` on the whole enum and apply it only to individual methods that truly require the main thread, or update all existing callers (including `ShortcutStore`) to account for the new isolation.</violation>
</file>
<file name="VoiceInk/Transcription/Cloud/CloudTranscriptionService.swift">
<violation number="1" location="VoiceInk/Transcription/Cloud/CloudTranscriptionService.swift:41">
P1: Custom agent: **Flag Security Vulnerabilities**
`@unchecked Sendable` suppresses the compiler's concurrency safety checks, but the class contains no locks or other synchronization. The stored `ModelContext` is not `Sendable` and is accessed from the non-isolated `transcribe()` method via `getCustomDictionaryTerms()`, creating a data race when the service is shared across tasks. The `lazy var openAICompatibleService` is also not thread-safe. Either remove `@unchecked Sendable` and keep the service confined to a single actor, or add proper synchronization and avoid storing a non-sendable `ModelContext`.</violation>
</file>
<file name="VoiceInk/Views/Recorder/RecorderComponents.swift">
<violation number="1" location="VoiceInk/Views/Recorder/RecorderComponents.swift:332">
P2: Custom agent: **Backward compatibility**
The new inline mode row in `RecorderModeButton` is hard-capped at 4 modes (`inlineModeLimit = 4`) and replaces the previous `ModePopover` that showed every enabled mode. Users with 5+ enabled modes can no longer switch to extra modes with the mouse in the recorder panel; only undiscoverable keyboard shortcuts reach them. This removes existing mouse-driven functionality and breaks backward compatibility.
Add an overflow affordance (e.g. a ‘More…’ chip that reopens `ModePopover`, or increase the inline limit with a scrollable row) so all enabled modes remain mouse-accessible from the recorder.</violation>
</file>
<file name="VoiceInk/Transcription/Streaming/ElevenLabsStreamingProvider.swift">
<violation number="1" location="VoiceInk/Transcription/Streaming/ElevenLabsStreamingProvider.swift:8">
P2: Concurrent provider calls can race the unchecked provider’s mutable state or its `ModelContext`; `@unchecked Sendable` suppresses Swift 6 diagnostics without providing the locking the new comment claims. Serialize access in an actor/lock, or actor-isolate the provider and protocol APIs.</violation>
</file>
<file name="VoiceInk/Transcription/Engine/VoiceInkEngine.swift">
<violation number="1" location="VoiceInk/Transcription/Engine/VoiceInkEngine.swift:632">
P2: A delayed result peek can reopen over a later recording and replace its live recorder UI. The callback needs a per-take lifecycle/generation check (and should be ignored once a newer take has started) rather than accepting every completion after the pipeline ID is cleared.</violation>
</file>
<file name="VoiceInk/Views/AudioPlayerView.swift">
<violation number="1" location="VoiceInk/Views/AudioPlayerView.swift:51">
P2: Long-recording waveforms still omit most audio between sample positions, despite the new peak calculation; aggregate every frame in each stride window (in chunks if needed) before assigning its bar.</violation>
<violation number="2" location="VoiceInk/Views/AudioPlayerView.swift:341">
P2: Dragging past either waveform edge creates negative or past-end trim timestamps; clamp gesture time to `0...duration` so the shown, previewed, and exported range match.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
| } | ||
|
|
||
| let outputFile = try AVAudioFile( | ||
| forWriting: destination, |
Contributor
There was a problem hiding this comment.
P1: Exporting over the source recording can destroy it: the writer opens destination directly even when it is source, truncating the input before the copy completes. Reject identical source/destination URLs (and preferably export to a temporary file before replacing a destination).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Services/AudioClipExporter.swift, line 81:
<comment>Exporting over the source recording can destroy it: the writer opens `destination` directly even when it is `source`, truncating the input before the copy completes. Reject identical source/destination URLs (and preferably export to a temporary file before replacing a destination).</comment>
<file context>
@@ -0,0 +1,106 @@
+ }
+
+ let outputFile = try AVAudioFile(
+ forWriting: destination,
+ settings: format.settings,
+ commonFormat: format.commonFormat,
</file context>
| func undoResultPeek() async { | ||
| let transcriptionID = resultPeek?.transcriptionID | ||
| dismissResultPeek() | ||
| _ = await CursorPaster.undoLastPaste() |
Contributor
There was a problem hiding this comment.
P1: A retry finishing after another take can undo the newer take and paste stale text. Confirm the peek's transcription is still latest before issuing Cmd+Z.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Transcription/Engine/VoiceInkEngine+Protocols.swift, line 38:
<comment>A retry finishing after another take can undo the newer take and paste stale text. Confirm the peek's transcription is still latest before issuing Cmd+Z.</comment>
<file context>
@@ -1,5 +1,115 @@
+ func undoResultPeek() async {
+ let transcriptionID = resultPeek?.transcriptionID
+ dismissResultPeek()
+ _ = await CursorPaster.undoLastPaste()
+
+ // Recorded rather than merely acted on: taking a result back is the strongest signal the
</file context>
Suggested change
| _ = await CursorPaster.undoLastPaste() | |
| guard LastTranscriptionService.getLastTranscription(from: modelContext)?.id == last.id else { return } | |
| _ = await CursorPaster.undoLastPaste() |
|
|
||
| /// Re-presents the panel to carry the result peek after delivery has dismissed it. | ||
| func presentPanelForResult() { | ||
| showRecorderPanel() |
Contributor
There was a problem hiding this comment.
P1: Result peek is shown while isRecorderPanelVisible remains false, so recorder shortcuts treat the visible peek as dismissed and can start a new take. Set visibility through the state property so observers and shortcut routing stay consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Transcription/Engine/RecorderUIManager.swift, line 90:
<comment>Result peek is shown while `isRecorderPanelVisible` remains false, so recorder shortcuts treat the visible peek as dismissed and can start a new take. Set visibility through the state property so observers and shortcut routing stay consistent.</comment>
<file context>
@@ -75,6 +85,11 @@ class RecorderUIManager: ObservableObject, RecorderPanelPresenting {
+ /// Re-presents the panel to carry the result peek after delivery has dismissed it.
+ func presentPanelForResult() {
+ showRecorderPanel()
+ }
+
</file context>
Suggested change
| showRecorderPanel() | |
| isRecorderPanelVisible = true |
|
|
||
| var ratios: [Double] = [] | ||
| for transcript in transcripts { | ||
| guard let enhanced = transcript.enhancedText, !enhanced.isEmpty, |
Contributor
There was a problem hiding this comment.
P1: Failed AI passes are counted as enhanced rewrites because their persisted error message is nonempty enhancedText, so repeated provider failures can make the dashboard report a large rewrite rate. Carry successful-enhancement state/duration into TranscriptFact and require it here before incrementing enhancement metrics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Views/Dashboard/DictationInsightFacts.swift, line 108:
<comment>Failed AI passes are counted as enhanced rewrites because their persisted error message is nonempty `enhancedText`, so repeated provider failures can make the dashboard report a large rewrite rate. Carry successful-enhancement state/duration into `TranscriptFact` and require it here before incrementing enhancement metrics.</comment>
<file context>
@@ -0,0 +1,383 @@
+
+ var ratios: [Double] = []
+ for transcript in transcripts {
+ guard let enhanced = transcript.enhancedText, !enhanced.isEmpty,
+ !transcript.rawText.isEmpty
+ else { continue }
</file context>
| var allTimePeakHours: DashboardPeakHoursSummary = .empty | ||
| /// Derived over the last 30 days — recent enough to describe how you work now, long enough to | ||
| /// have something behind it. | ||
| var insights: DashboardInsightBundle = .empty |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Backward compatibility
Adding a new non-optional Codable property to DashboardStatsSummary breaks loading of previously persisted dashboard snapshots. The struct is encoded to JSON via DashboardStatsSnapshotStore, and synthesized Swift Decodable does not respect default property values — older JSON files missing the insights key will throw keyNotFound on decode and cause the cache to return nil. This forces a full recalculation for existing users after upgrade.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Views/Dashboard/DashboardStatsModels.swift, line 280:
<comment>Adding a new non-optional `Codable` property to `DashboardStatsSummary` breaks loading of previously persisted dashboard snapshots. The struct is encoded to JSON via `DashboardStatsSnapshotStore`, and synthesized Swift `Decodable` does not respect default property values — older JSON files missing the `insights` key will throw `keyNotFound` on decode and cause the cache to return `nil`. This forces a full recalculation for existing users after upgrade.</comment>
<file context>
@@ -275,6 +275,9 @@ struct DashboardStatsSummary: Codable, Equatable, Sendable {
var allTimePeakHours: DashboardPeakHoursSummary = .empty
+ /// Derived over the last 30 days — recent enough to describe how you work now, long enough to
+ /// have something behind it.
+ var insights: DashboardInsightBundle = .empty
}
</file context>
| final class ElevenLabsStreamingProvider: StreamingTranscriptionProvider { | ||
| /// Conforms to a `Sendable` protocol: instances are handed between the engine's isolation | ||
| /// domains, and internal mutable state is guarded by locks or confined to one task. | ||
| final class ElevenLabsStreamingProvider: StreamingTranscriptionProvider, @unchecked Sendable { |
Contributor
There was a problem hiding this comment.
P2: Concurrent provider calls can race the unchecked provider’s mutable state or its ModelContext; @unchecked Sendable suppresses Swift 6 diagnostics without providing the locking the new comment claims. Serialize access in an actor/lock, or actor-isolate the provider and protocol APIs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Transcription/Streaming/ElevenLabsStreamingProvider.swift, line 8:
<comment>Concurrent provider calls can race the unchecked provider’s mutable state or its `ModelContext`; `@unchecked Sendable` suppresses Swift 6 diagnostics without providing the locking the new comment claims. Serialize access in an actor/lock, or actor-isolate the provider and protocol APIs.</comment>
<file context>
@@ -3,7 +3,9 @@ import LLMkit
-final class ElevenLabsStreamingProvider: StreamingTranscriptionProvider {
+/// Conforms to a `Sendable` protocol: instances are handed between the engine's isolation
+/// domains, and internal mutable state is guarded by locks or confined to one task.
+final class ElevenLabsStreamingProvider: StreamingTranscriptionProvider, @unchecked Sendable {
private let client = LLMkit.ElevenLabsStreamingClient()
</file context>
| guard let self else { return } | ||
|
|
||
| // Delivery has already dismissed the panel; bring it back holding the result. | ||
| self.recorderUIManager?.presentPanelForResult() |
Contributor
There was a problem hiding this comment.
P2: A delayed result peek can reopen over a later recording and replace its live recorder UI. The callback needs a per-take lifecycle/generation check (and should be ignored once a newer take has started) rather than accepting every completion after the pipeline ID is cleared.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Transcription/Engine/VoiceInkEngine.swift, line 632:
<comment>A delayed result peek can reopen over a later recording and replace its live recorder UI. The callback needs a per-take lifecycle/generation check (and should be ignored once a newer take has started) rather than accepting every completion after the pipeline ID is cleared.</comment>
<file context>
@@ -579,6 +621,17 @@ class VoiceInkEngine: NSObject, ObservableObject {
+ guard let self else { return }
+
+ // Delivery has already dismissed the panel; bring it back holding the result.
+ self.recorderUIManager?.presentPanelForResult()
+ self.presentResultPeek(peek)
+ },
</file context>
|
|
||
| if dependenciesChanged { | ||
| installChangeObservers() | ||
| } |
Contributor
There was a problem hiding this comment.
P2: Custom agent: Backward compatibility
deinit does not cancel snapshotObserver, so the underlying ObservationBridge observation remains registered with the observed @Observable services after the store deallocates. This is a regression from the previous Combine-based implementation, where AnyCancellable subscriptions were automatically torn down on dealloc. Every other class using ObservationBridge in this codebase calls cancel() in deinit (e.g., AIService and RecorderPanelShortcutManager). Adding snapshotObserver?.cancel() to deinit restores consistent lifecycle cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Modes/ModeFormWarmupStore.swift, line 121:
<comment>`deinit` does not cancel `snapshotObserver`, so the underlying `ObservationBridge` observation remains registered with the observed `@Observable` services after the store deallocates. This is a regression from the previous Combine-based implementation, where `AnyCancellable` subscriptions were automatically torn down on dealloc. Every other class using `ObservationBridge` in this codebase calls `cancel()` in `deinit` (e.g., `AIService` and `RecorderPanelShortcutManager`). Adding `snapshotObserver?.cancel()` to `deinit` restores consistent lifecycle cleanup.</comment>
<file context>
@@ -118,9 +120,7 @@ final class ModeFormWarmupStore: ObservableObject {
if dependenciesChanged {
installChangeObservers()
- }
</file context>
| // waveform is a point sample rather than an envelope and under-reports | ||
| // loud passages. | ||
| var peak: Float = 0 | ||
| for frame in 0..<Int(buffer.frameLength) { |
Contributor
There was a problem hiding this comment.
P2: Long-recording waveforms still omit most audio between sample positions, despite the new peak calculation; aggregate every frame in each stride window (in chunks if needed) before assigning its bar.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Views/AudioPlayerView.swift, line 51:
<comment>Long-recording waveforms still omit most audio between sample positions, despite the new peak calculation; aggregate every frame in each stride window (in chunks if needed) before assigning its bar.</comment>
<file context>
@@ -42,7 +44,14 @@ class WaveformGenerator {
+ // waveform is a point sample rather than an envelope and under-reports
+ // loud passages.
+ var peak: Float = 0
+ for frame in 0..<Int(buffer.frameLength) {
+ peak = max(peak, abs(channelData[frame]))
+ }
</file context>
| guard !isLoading, duration > 0 else { return } | ||
|
|
||
| hoverLocation = value.location.x | ||
| let time = Double(value.location.x / max(width, 1)) * duration |
Contributor
There was a problem hiding this comment.
P2: Dragging past either waveform edge creates negative or past-end trim timestamps; clamp gesture time to 0...duration so the shown, previewed, and exported range match.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At VoiceInk/Views/AudioPlayerView.swift, line 341:
<comment>Dragging past either waveform edge creates negative or past-end trim timestamps; clamp gesture time to `0...duration` so the shown, previewed, and exported range match.</comment>
<file context>
@@ -161,124 +176,187 @@ private func formatTime(_ time: TimeInterval) -> String {
+ guard !isLoading, duration > 0 else { return }
+
+ hoverLocation = value.location.x
+ let time = Double(value.location.x / max(width, 1)) * duration
+
+ if dragAnchor == nil {
</file context>
Suggested change
| let time = Double(value.location.x / max(width, 1)) * duration | |
| let time = min(max(0, Double(value.location.x / max(width, 1)) * duration), duration) |
Author
|
Closing this in favour of three smaller PRs. This was 216 files across three unrelated concerns, which is an unreasonable ask for a reviewer. Split into a stack, each building and passing tests on its own:
They stack rather than sit side by side: 2 needs the Observation migration, and 3 plumbs through a type 2 introduces. Both are pushed and ready — I will open them as PRs once #867 lands, so there is only ever one to review. Every commit was cherry-picked with Start with #867. It is the dullest of the three by design — no behaviour changes — and it is what the other two are built on. |
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.
Recorder redesign, Observation / Swift 6 migration, and an insights dashboard
This branch does three things that grew out of one another: it modernises the
codebase's concurrency and observation, rebuilds the recorder around a new
"ambient" surface, and turns the dashboard from a set of counters into something
that answers questions.
It is large — 215 files, ~10.6k lines — and I would completely understand a
request to split it. A suggested split is at the bottom, along with the parts I
think are least defensible. Everything here builds clean, passes 155 tests, and
merges into
mainwithout conflicts as ofupstream/main@dfe88ad.1. Observation and Swift 6 strict concurrency
ObservableObject/@Published→@Observableacross 38 types, and strictconcurrency turned on.
This is the change with the widest blast radius and the least visible payoff, so
it is worth saying what it actually bought. Two crashes in this repo were
concurrency bugs that strict checking makes impossible to write:
AudioDeviceManagertrapped on every audio device change. A CoreAudioproperty listener fires on a HAL queue and called into main-actor state, which
under Swift 6 is a hard trap rather than a race. Fixed by making the listener
nonisolatedand hopping explicitly._postWindowNeedsUpdateConstraintswhen a window was released while aGraphHosttransaction was still queued.Observation also removed a real performance problem — see §4.
Where the escape hatches are used they are commented with the reason.
nonisolated(unsafe)appears in exactly one place on purpose (Task.cancel()from a
deinit, which is documented safe from any thread).2. Ambient recorder
A third recorder style alongside Mini and Notch, where the display border is
the instrument and there is no panel at all.
The governing rule is that the frame never says two things at once: an arbiter
picks the single most important state and the whole border says only that, so
there is no legend to learn. Colour carries what, thickness carries how loud.
rather than floating under it, tracing the notch silhouette across the middle
and settling to the depth of the side wash at the corners. Newest audio sits
at the centre and older audio is pushed outward, so speech appears to be
emitted by the hardware.
live meter and becomes the recording being read back, lit outward as work
completes. Determinate when there is prediction history for the model,
honestly indeterminate when there is not.
brightness, so the light-background scheme inverts the approach entirely:
deeper colours, tighter blur, a contour that carries the shape. Colours were
chosen by measured contrast, not by eye — see §5.
default and accepts them only while something is genuinely clickable.
3. Dashboard insights
The dashboard counted words, minutes and sessions. Those go up, and knowing they
went up changes nothing. Each insight added here exists because there is a
decision behind it.
Presented as an overview of summary cards, each opening its own detail.
Three fields were added to
SessionMetricto support this(
targetBundleIdentifier,wasUndone,dictionaryHitCount). All optional ordefaulted, so SwiftData migrates in place; existing rows carry nils and the UI
distinguishes "not measured" from "measured zero".
Two notes on the arithmetic, both of which are tested:
two-word correction must not weigh the same as a five-minute dictation.
a decision, not a malfunction.
Nothing renders below a threshold of takes. A confident number computed from
three samples is worse than no number, because it teaches people to distrust the
rest of the screen.
4. Performance
Profiled with
sampleover live takes rather than estimated.The ambient surface put 16.8% of one core in the SwiftUI render path, of
which only 3.5% was drawing — 12.2% was
AG::Graph::UpdateStack::update,SwiftUI walking its graph 30 times a second to rediscover that only the waveform
had moved. That is a scope problem: audio-rate state lived on the view that
builds the whole surface.
Moving it to an
@Observablemodel read only by the leaf that draws it tookAttributeGraph to 5.7%. It also removed a stall — before the change, 245
samples were the main thread blocked in
RB::SurfacePool::wait_image_queue,starved of render surfaces by redundant frames. Idle cost is zero.
Measured on a
-Ononelocal build, so these are ceilings.5. Tests
17 lines of template → 155 tests. They cover the pure logic, and they are
weighted toward the things that actually broke during development rather than
toward coverage:
0.41; room tone stays under 0.15).
and 20 ΔE separation under simulated deuteranopia. The light scheme was
wrong twice before this test existed; it averaged 3.4:1 on white.
watchdog's next tick.
make testruns them.Merge with upstream
upstream/mainlanded a license/keychain change while this was in flight;fourteen files conflicted along the same seam. Upstream's semantics were kept
wholesale and re-expressed in this branch's idiom, never the reverse. Three
upstream additions needed adjusting to compile under strict concurrency (a
mutable static cache moved into an actor, a nonisolated
deinit, anon-
SendableUserDefaults). Details are in the merge commit.Reviewing this
Commits are self-contained and each explains why rather than what. Suggested
reading order:
refactor: migrate to Observation and enable Swift 6 strict concurrencyAmbientRecorderView,AmbientVoiceCrest,AmbientPresentation)perf(ambient): narrow render scopeWhat I would push back on myself
migration, the ambient recorder, and the dashboard. They are separable, and I
am happy to split them.
figure is measured; that one is not. It is labelled, but it is still a
hardcoded constant in a screen otherwise built from real data.
cannot be backfilled, so those cards are empty until new takes accumulate.
tested; how the surface actually looks and behaves during a take is not, and
most bugs found during development were visual and invisible to tests.
RecorderUIManageris still untestable — it cannot be constructed withoutthe engine, so the panel-teardown ordering that caused a real bug is verified
by reading rather than by a test.
Summary by cubic
Adds an insights overview to the dashboard with six linked cards and drill‑in detail; also migrates the app to Swift 6 + Observation and reconciles upstream license/keychain changes under strict concurrency.
New Features
Migration
@Observableacross the app; explicitSendable/nonisolatedwhere needed; newObservationBridgefor derived state.make localto preserve Accessibility/Screen Recording/Microphone grants across rebuilds; add-skipPackagePluginValidationformlx-swift; updated guidance in BUILDING.md.LicenseViewModelsemantics while remaining@Observable; madeLicenseStoring/PolarServicingSendable; annotated thread‑safe services (e.g., keychain/storage) for Swift 6.Written for commit 2dc4429. Summary will update on new commits.
🤖 Generated with Claude Code
https://claude.ai/code/session_014RALpihnmTsYkJ3EkA1G5q