Speakers: release held people on confirm, floor the separation folds - #1928
Merged
Merged
Conversation
- A review confirmation now releases a held voiceprint-migration person in the same transaction, instead of waiting for the next launch's run. Unreadable held ledger rows are logged as a count. - Separation step 1 folds a short voice only when it clears the model's microAbsorb bar; a voice with no fingerprint is never folded. - The invite cap folds a voice only when it clears separationMerge, so a 1:1 invite can't collapse a third person into the invitee. - The separation provider reads the diarizer's active backend per meeting, so a Nemotron load failure gets pyannote's settings.
Owner
Author
|
Independent review (coordinator session, full Sources diff vs main): APPROVE.
|
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.
Four speaker-naming bugs from code review. Three of them showed up on real hardware.
1. Held people never released during a session
Bug:
releaseHeldVoiceprintConfirmations()only ran at the start ofSpeakerVoiceprintMigration.run(), once per launch. A review confirmation wrotespeaker_profile_confirmationsbut never touched the ledger, so a held person needed a manual confirm in every meeting until the next launch. On hardware, a colleague from 216 calls needed one every time.Fix: The release rules moved into
releaseHeldVoiceprintConfirmationsImpl(holders:).recordUserConfirmationsImplcalls it after its inserts, in the same transaction, for the profiles it just confirmed. The rules themselves didn't change: follow merges to the holder, the holder must exist and have a confirmation, then carry the held confirmations over and mark the row released. Only the timing changed. The pass is skipped when the database has no migration ledger. The launch run still does its full pass as a safety net. A held row that fails to decode is now logged as a count only (unreadable_count), with no names.Tests (
SpeakerVoiceprintMigrationTests):testAudioThatDisagreesKeepsTheNameButWaitsForOneConfirmationnow expects the release right after the confirmation, and expects the next run to release nobody.testConfirmingSomeoneElseReleasesNobodytestConfirmingThePersonAHeldProfileWasMergedIntoReleasesIttestAnUnreadableHeldRowStaysHeldAndDoesNotBlockTheConfirmation2. Tiny voices folded with no similarity floor
Bug: Step 1 of
SpeakerSeparation.applyfolded every voice under 5 s into a plain argmax, with no minimum similarity. A voice with no fingerprint went to whoever talked most. On hardware, a real 3.4 s line from a different person got someone else's label.Fix: New
SpeakerSeparationOptions.foldSimilarity. Step 1 now folds only when the cosine to the target is at least that bar, and it never folds a voice that has no fingerprint. Both tuned presets set the bar to the active voiceprint model'smicroAbsorb: 0.62 for WeSpeaker, 0.626 for ReDimNet2, 0.45 for ERes2Net. I pickedmicroAbsorbbecause it's the clusterer's existing bar for "absorb a very short cluster (<10 s) into another". That's the same decision as this step, and it's calibrated per model.Tests (
SpeakerSeparationTests):testANearSilentVoiceWithNoFingerprintStaysItsOwnSpeakerreplaces the old test that folded it into the loudest voice.testAShortLineFromSomeoneElseKeepsItsOwnSpeaker(the 3.4 s hardware case)testAShortPieceOfSomeoneWhoTalkedMoreStillFoldstestAShortVoiceFoldsOnlyWhenItClearsTheFoldBar3. A 1:1 invite capped the call side to one voice
Bug:
nemotronTunedsetsmaxSpeakers: 1for a one-person invite. The cap step then folded every other voice into the loudest one with no similarity check. So a recurring 1:1 slot that held a different call, or a third person joining, collapsed everyone into one named speaker.Fix: New
capFoldSimilarity, set to the model'sseparationMerge(0.6 WeSpeaker, 0.598 ReDimNet2). The cap step folds the quietest voice that clears the bar. A voice that clears nobody stays, even if that leaves more voices than the cap. With no bar set, the old behavior is unchanged.Tests:
testAOneOnOneCapNeverCollapsesADifferentPersonIntoTheInviteetestAOneOnOneWithOnlyTheInviteeStillEndsWithOneVoice(the case the cap exists for still works)testACapWithABarKeepsAVoiceWithNoFingerprinttestFoldAndCapBarsComeFromTheActiveVoiceprintModel4. Nemotron fallback kept Nemotron options
Bug:
speakerSeparationProvidercaptureddiarizationBackendwhen the app started. If Nemotron failed to load, pyannote ran with Nemotron-tuned separation.Fix: A new
MeetingSpeakerSeparationProvider.makebuilds the provider. It readsdiarizer.activeBackendon every call, and the pipeline makes that call after the models load. TheMeetingSessionControlleredit is minimal, andcheck-source-pins --changed-onlypasses.Tests: new fast test
Tests/MeetingSpeakerSeparationProviderTests.swift. It flips the active backend after the provider is built and checks that the next meeting gets the new backend and its recording date. The new file is added to therun-tests.shsource list.Judgment call to review
labTunedalso setsfoldSimilarity = microAbsorbandcapFoldSimilarity = separationMerge. This matters because after fix 4, pyannote is exactly what a Nemotron fallback runs, and its 1:1 cap would otherwise collapse people the same way. The cost: pyannote's step 2 already merges everything atseparationMerge, so its invite cap is now close to a no-op. The YODAS lab credited that cap with part of pyannote's 1:1 gain. Pyannote is only the fallback now. An extra row takes one click to merge in review; two people under one name can't be split. If you'd rather keep pyannote's cap as it was, dropcapFoldSimilarityfromlabTuned.Checks
bash build-deps.sh --force, thenbash build.sh --no-open: buildsswift test --filter SpeakerTests: 522 tests, 0 failures (14 skipped)bash run-tests.sh --filter MeetingSpeakerSeparationProviderTests: 4/4python3 scripts/dev/check-source-pins.py --changed-only: passbash check.sh: pass (15/15)Still needed before merge: an independent review of the full diff (per AGENTS.md), and a hardware re-check of the 1:1 and 3.4 s cases.
🤖 Generated with Claude Code