SpeakerManager improvements by SGD2718 · Pull Request #180 · FluidInference/FluidAudio

SGD2718 · 2025-11-06T02:24:39Z

I added some new features to the SpeakerManager class:

Merging within the database
Speaker removal
Permanent speakers (they cannot be removed without an override in a function parameter)
Known speaker initialization when the database has speakers in it already that might overlap
Speaker filtering
Similar speaker detection

I also updated the documentation in SpeakerManager.md to reflect these new features.

This is the first time I've made a PR to a public repository, so I'm not sure what else I need to do.

…izing known speakers

Alex-Wengg · 2025-11-06T02:25:41Z

hi @SGD2718 , your pr details make sense here its fine.

Alex-Wengg

so far so good its a good start but i have left some comments that need to be addressed

Documentation/SpeakerManager.md

Alex-Wengg · 2025-11-06T03:09:38Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

we could use some unit tests for these new functions such as these cases

// Returns false (doesn't merge) if: guard sourceId != destinationId else { return false } // Same ID guard let speakerToMerge = ... else { return false } // Speaker doesn't exist guard !(stopIfPermanent && ...) else { return false } // Is permanent

Where do i write them?

Documentation/SpeakerManager.md

If I removed the return type from mergeSpeaker, do these examples still need to be added to the docs?

@SGD2718 apologies i misunderstood the context . sorry the unit tests should be in this path , it shouldn't be too difficult for an coding agent to just write them to cover all the cases

https://github.com/FluidInference/FluidAudio/tree/main/Tests/FluidAudioTests

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

Alex-Wengg · 2025-11-06T04:02:48Z

https://github.com/FluidInference/FluidAudio/actions/runs/19122767418/job/54646480813?pr=180

Copilot

Pull Request Overview

This pull request adds a comprehensive speaker permanence feature to the diarization system, allowing certain speakers to be protected from automatic merging and removal operations. The update also improves performance in the embedding quality calculation and adds extensive new APIs for speaker management.

Adds isPermanent flag to Speaker class to protect speakers from automatic operations
Introduces SpeakerInitializationMode enum with reset/merge/overwrite/skip options for handling duplicate speaker IDs
Expands SpeakerManager API with 15+ new methods for finding, merging, and removing speakers with permanence controls
Optimizes embedding quality calculation using vDSP.sumOfSquares instead of manual map-reduce

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 21 comments.

File	Description
Sources/FluidAudio/Diarizer/Core/DiarizerManager.swift	Optimized `calculateEmbeddingQuality` to use Accelerate's `vDSP.sumOfSquares` for better performance
Sources/FluidAudio/Diarizer/Clustering/SpeakerTypes.swift	Added `isPermanent` flag to `Speaker` class and introduced `SpeakerInitializationMode` enum for controlling speaker initialization behavior
Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift	Extensive API expansion with methods for speaker permanence management, finding/merging/removing speakers, and enhanced initialization with conflict resolution modes
Documentation/SpeakerManager.md	Comprehensive documentation updates covering all new APIs, initialization modes, permanence features, and usage examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Documentation/SpeakerManager.md

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

Documentation/SpeakerManager.md

Copilot · 2025-11-06T05:28:53Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

+    }
+
+    /// Remove a speaker's permanent marker
+    /// - Parameter speakerId: the ID of the speaker to mark as permanent


Incorrect parameter description: the documentation says "the ID of the speaker to mark as permanent" but this method removes the permanent marker, not adds it. Should say: "the ID of the speaker to remove the permanent marker from"

Suggested change

/// - Parameter speakerId: the ID of the speaker to mark as permanent

/// - Parameter speakerId: the ID of the speaker to remove the permanent marker from

Copilot · 2025-11-06T05:28:53Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

+    ///    - excludeIfBothPermanent: whether to exclude speaker pairs where both speakers are permanent
+    /// - Returns: a list of speaker ID pairs that belong to speakers that are similar enough to be merged
+    public func findMergeablePairs(speakerThreshold: Float? = nil, excludeIfBothPermanent: Bool = true) -> [(speakerToMerge: String, destination: String)] {
+        queue.sync(flags: .barrier) {


Using .barrier flag with queue.sync for a read-only operation. The findMergeablePairs method only reads from speakerDatabase and doesn't modify it, so it should use queue.sync without the .barrier flag for better concurrency. The .barrier flag is meant for write operations that need exclusive access.

Suggested change

queue.sync(flags: .barrier) {

queue.sync {

Copilot · 2025-11-06T05:28:53Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

+        }
+    }
+
+    /// remove non-permanent speakers that meet a certain predicate


Inconsistent capitalization: method name "remove non-permanent speakers" should start with a capital letter to be consistent with other method documentation. Should be: "Remove non-permanent speakers that meet a certain predicate"

Suggested change

/// remove non-permanent speakers that meet a certain predicate

/// Remove non-permanent speakers that meet a certain predicate

Copilot · 2025-11-06T05:28:54Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerTypes.swift

    /// Update main embedding with new segment data using exponential moving average
+    /// - Parameters:
+    ///   - duration: segment duration
+    ///   - 


Incomplete parameter documentation. The - is present but the parameter description is missing. This should describe what the embedding parameter is.

Suggested change

/// -

/// - embedding: The new embedding vector for the speaker segment

Documentation/SpeakerManager.md

…alization

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Alex-Wengg · 2025-11-06T17:59:18Z

https://github.com/FluidInference/FluidAudio/actions/runs/19127380127/job/54709369614?pr=180

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift:154

The assignSpeaker function accepts newName parameter (line 121) but doesn't pass it to createNewSpeaker. This means the custom name provided by callers will be ignored when creating new speakers.

                let newSpeakerId = createNewSpeaker(
                    embedding: normalizedEmbedding,
                    duration: speechDuration,
                    distanceToClosest: distance
                )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Documentation/SpeakerManager.md

Alex-Wengg · 2025-11-08T04:34:19Z

Looks like the tests finally passed haha. Just that annoying format error

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

SGD2718 · 2025-11-08T05:15:37Z

There we go

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

Documentation/SpeakerManager.md

Copilot · 2025-11-08T05:22:58Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

+                            logger.warning(
+                                "Failed to overwrite Speaker \(speaker.id) because it is permanent. Skipping")
+                        }


When the overwrite is skipped due to permanence, the code should use continue to skip the rest of the loop iteration. Without it, the code proceeds to line 89-96, which incorrectly tracks the numeric ID and logs "Initialized known speaker" even though the speaker was not actually added or updated. Add continue after line 70.

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

Documentation/SpeakerManager.md

Copilot · 2025-11-08T05:22:59Z

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

                existingSpeaker.rawEmbeddings = rawEmbeddings
                existingSpeaker.updateCount = updateCount
                existingSpeaker.updatedAt = updatedAt ?? now
+                existingSpeaker.isPermanent = isPermanent


When updating an existing speaker, unconditionally setting isPermanent to the parameter value (which defaults to false) can inadvertently remove the permanent flag from speakers that were previously marked as permanent. Consider preserving the existing isPermanent value unless explicitly provided, or document this behavior clearly to prevent accidental removal of permanence.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift:156

The newName parameter accepted by assignSpeaker (line 123) is not passed to createNewSpeaker, which has a name parameter (line 481). This means the caller cannot specify a custom name for newly created speakers, making the parameter non-functional.

                let newSpeakerId = createNewSpeaker(
                    embedding: normalizedEmbedding,
                    duration: speechDuration,
                    distanceToClosest: distance
                )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Documentation/SpeakerManager.md

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

SGD2718 · 2025-11-08T06:15:39Z

Alright I think that that should be all the formatting issues

SGD2718 · 2025-11-08T06:32:17Z

Finally

Alex-Wengg · 2025-11-08T18:05:49Z

@SGD2718 looks good to me great job on your first PR !

SGD2718 · 2025-11-08T20:36:37Z

Do i close the PR or do you do that?

Alex-Wengg · 2025-11-09T04:46:10Z

@SGD2718 you can merge it, lmk if you are unable to merge the pr

SGD2718 · 2025-11-09T04:54:38Z

I don't think I can merge it. My earlier question was due to my getting confused about what the close with comment button did.

Alex-Wengg · 2025-11-09T13:09:23Z

I will merge it , the close with comment would cause the pr to be cancelled

SGD2718 added 6 commits November 2, 2025 19:39

Vectorized embedding quality calculation

3b7fb76

Added SpeakerInitializationMode enum to dictate behavior when initial…

80aa250

…izing known speakers

Implemented a lot more features for finer control

8978171

Added speaker permanence and merge detection

aa0f95e

Merge branch 'FluidInference:main' into main

82c7239

Updated Documentation

ac05a63

Copilot AI review requested due to automatic review settings November 6, 2025 02:24

Alex-Wengg requested review from Alex-Wengg and removed request for Copilot November 6, 2025 02:25

Alex-Wengg reviewed Nov 6, 2025

View reviewed changes

BrandonWeng assigned Alex-Wengg Nov 6, 2025

BrandonWeng added enhancement New feature or request speaker-diarization Issues related to speaker diarization labels Nov 6, 2025

Copilot AI review requested due to automatic review settings November 6, 2025 05:20

SGD2718 force-pushed the main branch from 54aed98 to 1d8989f Compare November 6, 2025 05:26

Copilot AI reviewed Nov 6, 2025

View reviewed changes

Improved Concurrency Safety & fixed typos in documentation

a976c9c

SGD2718 force-pushed the main branch from 1d8989f to acd5823 Compare November 6, 2025 05:38

Fixed capitalization in docstrings

c00d1e9

SGD2718 force-pushed the main branch from acd5823 to c00d1e9 Compare November 6, 2025 05:41

Fixed issue where merging did not respect permanent speakers in initi…

44d5b55

…alization

SGD2718 force-pushed the main branch from ac81916 to 44d5b55 Compare November 6, 2025 05:59

SGD2718 and others added 3 commits November 5, 2025 22:03

fixed typos

f9a48a0

Update Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

bacd0b1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'main' into main

35abfb2

SGD2718 added 2 commits November 6, 2025 10:27

fixed a typo

545d561

Merge branch 'main' into main

4c822e6

SGD2718 requested a review from Alex-Wengg November 8, 2025 04:06

Copilot AI reviewed Nov 8, 2025

View reviewed changes

Documentation/SpeakerManager.md Outdated Show resolved Hide resolved

Update Documentation/SpeakerManager.md

432ee7c

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

SGD2718 requested a review from Copilot November 8, 2025 05:18

Copilot AI reviewed Nov 8, 2025

View reviewed changes

SGD2718 and others added 6 commits November 7, 2025 21:35

Update Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

de77a38

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update Sources/FluidAudio/Diarizer/Clustering/SpeakerManager.swift

7204b4d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update Documentation/SpeakerManager.md

e9ea4d2

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update Documentation/SpeakerManager.md

4b60a44

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fixed Minor bugs

125c9af

Merge remote-tracking branch 'refs/remotes/origin/main'

da57fc4

SGD2718 requested a review from Copilot November 8, 2025 05:48

Copilot AI reviewed Nov 8, 2025

View reviewed changes

Documentation/SpeakerManager.md Outdated Show resolved Hide resolved

SGD2718 and others added 3 commits November 7, 2025 21:53

Update Documentation/SpeakerManager.md

4a07829

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Fixed Formatting Issues (I think)

a6b99fd

Merge remote-tracking branch 'refs/remotes/origin/main'

e443a66

Alex-Wengg approved these changes Nov 8, 2025

View reviewed changes

Alex-Wengg merged commit 3985a4e into FluidInference:main Nov 9, 2025
10 checks passed

Alex-Wengg pushed a commit that referenced this pull request Jan 1, 2026

SpeakerManager improvements (#180)

b41c0c2

SGD2718 added a commit that referenced this pull request Jan 4, 2026

SpeakerManager improvements (#180)

2e00370

Alex-Wengg pushed a commit that referenced this pull request Jan 5, 2026

SpeakerManager improvements (#180)

d6402bd

	/// - Parameter speakerId: the ID of the speaker to mark as permanent
	/// - Parameter speakerId: the ID of the speaker to remove the permanent marker from

	/// remove non-permanent speakers that meet a certain predicate
	/// Remove non-permanent speakers that meet a certain predicate

	/// -
	/// - embedding: The new embedding vector for the speaker segment

Conversation

SGD2718 commented Nov 6, 2025

Uh oh!

Alex-Wengg commented Nov 6, 2025

Uh oh!

Alex-Wengg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alex-Wengg Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

SGD2718 Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Alex-Wengg Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

SGD2718 Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Alex-Wengg Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Alex-Wengg commented Nov 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Alex-Wengg commented Nov 6, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Alex-Wengg commented Nov 8, 2025

Uh oh!

SGD2718 commented Nov 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview