feat(app): re-open speaker naming on finished jobs (late re-naming recovery)#337
Open
pasrom wants to merge 1 commit into
Open
feat(app): re-open speaker naming on finished jobs (late re-naming recovery)#337pasrom wants to merge 1 commit into
pasrom wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #337 +/- ##
=======================================
+ Coverage 79.8% 79.9% +0.1%
=======================================
Files 89 89
Lines 9074 9123 +49
=======================================
+ Hits 7246 7297 +51
+ Misses 1828 1826 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Adds a recovery path for the speaker-naming auto-confirm bug: the dialog sometimes confirms before the user names speakers (a stray Enter/Cmd-W while another app holds focus). PR #310 shipped prevention (a 750 ms keyboard-shortcut grace); this adds the recovery that prevention can't fully cover. A finished (.done) meeting that retains its naming data now shows an "Edit Speaker Names" button in the menu-bar job row, which re-opens the naming dialog so the user can name speakers — or re-run diarization — after the fact. What: - Retain the naming sidecar: Confirm/Skip free only the in-RAM caches (clearNamingCaches); the persisted _naming.json + audio sidecars stay on disk. Full deletion (removeNamingData) runs only on explicit Dismiss or cancel. This mirrors what the app already does with recordings — the original _mix/_app/_mic WAVs are kept in recordings/ indefinitely, so the small naming data lives alongside them rather than being reaped. - Don't auto-remove jobs that retain a sidecar (namingSlug != nil) so the affordance stays reachable; jobs without naming data keep the ~60 s auto-clear. - reopenSpeakerNaming(jobID:) reloads the data (RAM, else disk) and flips .done -> .speakerNamingPending via the existing .showSpeakerNaming path. - persistAppliedNames folds confirmed names into the sidecar so a second re-name anchors on the current transcript labels, not the auto-names. - Log recognition once per job (recognitionLoggedJobIDs) so a re-confirm doesn't write a phantom perfect-match stats row. Reasoning: - Recovery is in-session: .done jobs are dropped from the snapshot on relaunch, so the menu affordance doesn't survive a restart (the retained sidecar persists on disk for a future "recent recordings" surface). - Finished diarized meetings now persist in the menu until Dismiss — the deliberate tradeoff that keeps the re-name affordance reachable.
b2cec88 to
9a8a861
Compare
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a recovery path for the speaker-naming dialog auto-confirm bug: the dialog sometimes confirms before the user names speakers (a stray Enter/Cmd-W while another app holds focus). PR #310 shipped prevention (a 750 ms keyboard-shortcut grace); this PR adds the recovery that prevention can't fully cover.
A finished (
.done) meeting that retains its naming data now shows an "Edit Speaker Names" button in the menu-bar job row. Clicking it re-opens the naming dialog so the user can name speakers — or re-run diarization — after the fact.How it works
clearNamingCaches); the persisted_naming.json+ audio sidecars stay on disk. Full deletion (removeNamingData) happens only on explicit Dismiss or cancel. This mirrors what the app already does with every recording: the original_mix.wav/_app.wav/_mic.wavare kept inrecordings/indefinitely, so the (small) naming data simply lives alongside them rather than being reaped on its own..donejob that retains a sidecar (signalled by a non-nilnamingSlug) is no longer auto-removed from the menu aftercompletedJobLifetime, so the affordance stays reachable. Jobs that never produced naming data keep the original ~60 s auto-clear.reopenSpeakerNaming(jobID:)reloads the retained data (RAM, else from disk) and flips.done → .speakerNamingPending, reusing the existing.showSpeakerNamingpath. Confirming folds the applied names back into the sidecar (persistAppliedNames) so a second re-name anchors on the current transcript labels instead of the original auto-names.suggestedbaseline is the already-applied names (a phantom perfect match).Scope / limitations
.donejobs are dropped from the snapshot on relaunch, so the menu affordance does not survive an app restart. The retained sidecar stays on disk (alongside the recordings the app already keeps), so a future "recent recordings" browser could surface it — intentionally out of scope here.Tests
PipelineQueueTests+MenuBarViewTests: deferred-delete, Dismiss cleanup, auto-remove gating (retained vs not), reopen transition / no-op / RAM-vs-disk, applied-name persistence for repeat re-name, and the "Edit Speaker Names" button visibility + callback.PipelineQueueTestsandMenuBarViewTestsgreen; SwiftLint clean; release-build parity check passes.Follow-up (noted, not in this PR)
The retained
_16k.wavis a downsampled duplicate of the_mix.wavthe app already keeps forever;lateDiarizationcould regenerate it on demand from the original instead of retaining a separate copy, removing the duplicate entirely.