Skip to content

Commit d1b4fb8

Browse files
committed
refactor(speaker): pass the tenant to enrolment instead of parsing it out of an id
/enroll/upload and /enroll/batch received no user_id. The service recovered the tenant with int(speaker_id.split("_")[1]), so the id's `user_{id}_speaker_{hex}` shape decided which gallery a voiceprint entered. A naming convention is not an ownership record: it is absent for a client that names ids differently and wrong for an imported speaker, and nothing checked it against the caller. Both endpoints now take user_id as a form field, and every caller sends it: Chronicle's speaker client, the WebUI's Enrollment and Annotation pages and its api.ts helpers, enroll_speaker.py, laptop_client.py, the diarization-validation script, and the integration test. /enroll/append is unchanged and takes no user_id — it requires the speaker to exist, so the record answers the question and a supplied tenant could only disagree with it. extract_user_id_from_speaker_id is now owner_of_speaker: a lookup of speakers.user_id with no parsing left, 404 when the speaker is unknown. Its other callers (get/download/delete) all operate on speakers that exist. Speaker suite green; WebUI typechecks.
1 parent 22cd831 commit d1b4fb8

13 files changed

Lines changed: 234 additions & 78 deletions

File tree

backends/advanced/src/advanced_omi_backend/speaker_recognition_client.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,6 +1525,7 @@ async def enroll_new_speaker(
15251525
)
15261526
form_data.add_field("speaker_id", speaker_id)
15271527
form_data.add_field("speaker_name", speaker_name)
1528+
form_data.add_field("user_id", user_id)
15281529

15291530
async with session.post(
15301531
f"{self.service_url}/enroll/upload",
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
"""Validate the PRODUCTION diarization path: enroll speakers, then /diarize-and-identify, and
2+
score DER on the IDENTIFIED labels (identified_as) — which is what the backend actually stores.
3+
4+
This tests whether per-segment centroid matching reconciles the cross-chunk label inconsistency
5+
(the user's point). Enrollment segments are EXCLUDED from scoring via a held-out UEM, so it's not
6+
train-on-test. Throwaway user_id=9999; speakers deleted by the caller afterward.
7+
"""
8+
9+
import io
10+
import json
11+
import sys
12+
from pathlib import Path
13+
14+
import numpy as np
15+
import requests
16+
import soundfile as sf
17+
18+
SVC = "http://localhost:8085"
19+
SR = 16000
20+
USER = 9999
21+
N_ENROLL = 3 # segments per speaker used for enrollment
22+
MIN_ENROLL_S = 2.5 # only use segments at least this long for enrollment
23+
24+
25+
def parse_rttm(p: Path):
26+
by_spk = {}
27+
for ln in p.read_text().splitlines():
28+
f = ln.split()
29+
if f and f[0] == "SPEAKER":
30+
start, dur, spk = float(f[3]), float(f[4]), f[7]
31+
by_spk.setdefault(spk, []).append((start, start + dur))
32+
return by_spk
33+
34+
35+
def main():
36+
meeting = sys.argv[1]
37+
wav = Path(f"data/ami_sdm_slice/audio/{meeting}.wav")
38+
ref = Path(f"data/ami_sdm_slice/ref_rttm/{meeting}.rttm")
39+
outdir = Path("experiments/pyannote_diarization_validation/enrolled")
40+
outdir.mkdir(parents=True, exist_ok=True)
41+
uemdir = outdir / "uem"
42+
uemdir.mkdir(exist_ok=True)
43+
hypdir = outdir / f"{meeting}_results"
44+
hypdir.mkdir(exist_ok=True)
45+
46+
audio, sr = sf.read(wav, dtype="float32")
47+
assert sr == SR
48+
by_spk = parse_rttm(ref)
49+
50+
enroll_ranges = [] # (start,end) excluded from scoring
51+
for spk, segs in by_spk.items():
52+
longest = sorted(segs, key=lambda s: s[1] - s[0], reverse=True)
53+
picked = [s for s in longest if s[1] - s[0] >= MIN_ENROLL_S][:N_ENROLL]
54+
if not picked:
55+
picked = longest[:N_ENROLL]
56+
clip = np.concatenate([audio[int(s * SR) : int(e * SR)] for s, e in picked])
57+
enroll_ranges += picked
58+
buf = io.BytesIO()
59+
sf.write(buf, clip, SR, format="WAV", subtype="PCM_16")
60+
buf.seek(0)
61+
sid = f"user_{USER}_{meeting}_{spk}"
62+
r = requests.post(
63+
f"{SVC}/enroll/upload",
64+
files={"file": ("enroll.wav", buf, "audio/wav")},
65+
data={"speaker_id": sid, "speaker_name": spk, "user_id": str(USER)},
66+
timeout=120,
67+
)
68+
print(
69+
f"enroll {spk}: {r.status_code} ({sum(e-s for s,e in picked):.1f}s)",
70+
flush=True,
71+
)
72+
73+
# held-out UEM = whole meeting minus enrolled ranges
74+
total = len(audio) / SR
75+
ranges = sorted(enroll_ranges)
76+
held, cur = [], 0.0
77+
for s, e in ranges:
78+
if s > cur:
79+
held.append((cur, s))
80+
cur = max(cur, e)
81+
if cur < total:
82+
held.append((cur, total))
83+
(uemdir / f"{meeting}.uem").write_text(
84+
"\n".join(f"{meeting} 1 {s:.3f} {e:.3f}" for s, e in held) + "\n"
85+
)
86+
87+
# diarize + identify with enrolled speakers
88+
with open(wav, "rb") as f:
89+
r = requests.post(
90+
f"{SVC}/diarize-and-identify",
91+
files={"file": (wav.name, f, "audio/wav")},
92+
data={
93+
"user_id": str(USER),
94+
"min_duration": 0.5,
95+
"collar": 2.0,
96+
"min_duration_off": 1.5,
97+
},
98+
timeout=1800,
99+
)
100+
segs = r.json().get("segments", [])
101+
n_id = sum(1 for s in segs if s.get("identified_as"))
102+
print(
103+
f"{meeting}: {len(segs)} segs, {n_id} identified, "
104+
f"{len({s['speaker'] for s in segs})} diar-labels",
105+
flush=True,
106+
)
107+
108+
# hyp speaker = identified_as (production label) else fall back to diar label
109+
hyp = [
110+
{
111+
"start": s["start"],
112+
"end": s["end"],
113+
"speaker": s.get("identified_as") or f"diar_{s.get('speaker')}",
114+
}
115+
for s in segs
116+
]
117+
json.dump(
118+
{"segments": hyp, "provider": "enrolled_identified"},
119+
open(hypdir / f"{meeting}.pyannote.json", "w"),
120+
)
121+
print(
122+
f"wrote {hypdir}/{meeting}.pyannote.json + held-out uem ({len(held)} regions)",
123+
flush=True,
124+
)
125+
126+
127+
if __name__ == "__main__":
128+
main()

extras/speaker-recognition/laptop_client.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,19 @@ async def health_check(self):
8282
logger.info(f"Health check response: {health_response}")
8383
return health_response
8484

85-
async def enroll_speaker(self, speaker_id: str, speaker_name: str, audio_path: str):
85+
async def enroll_speaker(
86+
self, speaker_id: str, speaker_name: str, user_id: str, audio_path: str
87+
):
8688
"""Enroll a speaker from audio file by uploading it."""
8789
if not self.session:
8890
raise RuntimeError("Client not initialized")
8991

9092
# Prepare query parameters
91-
params = {"speaker_id": speaker_id, "speaker_name": speaker_name}
93+
params = {
94+
"speaker_id": speaker_id,
95+
"speaker_name": speaker_name,
96+
"user_id": user_id,
97+
}
9298

9399
# Prepare file upload
94100
with open(audio_path, "rb") as f:
@@ -275,7 +281,7 @@ async def cmd_enroll(args):
275281

276282
# Enroll speaker
277283
result = await client.enroll_speaker(
278-
args.speaker_id, args.speaker_name, audio_path
284+
args.speaker_id, args.speaker_name, args.user_id, audio_path
279285
)
280286
logger.info(f"Server response: {result}")
281287

@@ -524,6 +530,9 @@ def main():
524530
# Enroll command
525531
enroll_parser = subparsers.add_parser("enroll", help="Enroll a new speaker")
526532
enroll_parser.add_argument("--speaker-id", required=True, help="Unique speaker ID")
533+
enroll_parser.add_argument(
534+
"--user-id", required=True, help="Chronicle user id owning the speaker"
535+
)
527536
enroll_parser.add_argument(
528537
"--speaker-name", required=True, help="Speaker display name"
529538
)

extras/speaker-recognition/scripts/enroll_speaker.py

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ def enroll_single_file(
7070
file_path: str,
7171
speaker_id: str,
7272
speaker_name: str,
73+
user_id: str,
7374
start: Optional[float] = None,
7475
end: Optional[float] = None,
7576
service_url: str = None,
@@ -84,7 +85,11 @@ def enroll_single_file(
8485
try:
8586
with open(file_path, "rb") as f:
8687
files = {"file": (os.path.basename(file_path), f, "audio/wav")}
87-
data = {"speaker_id": speaker_id, "speaker_name": speaker_name}
88+
data = {
89+
"speaker_id": speaker_id,
90+
"speaker_name": speaker_name,
91+
"user_id": user_id,
92+
}
8893

8994
if start is not None:
9095
data["start"] = start
@@ -118,7 +123,7 @@ def enroll_single_file(
118123

119124

120125
def enroll_multiple_files(
121-
file_paths: List[str], speaker_id: str, speaker_name: str
126+
file_paths: List[str], speaker_id: str, speaker_name: str, user_id: str
122127
) -> bool:
123128
"""Enroll speaker from multiple audio files for better accuracy."""
124129
valid_files = [f for f in file_paths if os.path.exists(f)]
@@ -138,7 +143,11 @@ def enroll_multiple_files(
138143
("files", (os.path.basename(file_path), content, "audio/wav"))
139144
)
140145

141-
data = {"speaker_id": speaker_id, "speaker_name": speaker_name}
146+
data = {
147+
"speaker_id": speaker_id,
148+
"speaker_name": speaker_name,
149+
"user_id": user_id,
150+
}
142151

143152
response = requests.post(
144153
f"{SPEAKER_SERVICE_URL}/enroll/batch", files=files, data=data
@@ -161,7 +170,9 @@ def enroll_multiple_files(
161170
return False
162171

163172

164-
def enroll_from_directory(directory: str, speaker_id: str, speaker_name: str) -> bool:
173+
def enroll_from_directory(
174+
directory: str, speaker_id: str, speaker_name: str, user_id: str
175+
) -> bool:
165176
"""Enroll speaker from all audio files in a directory."""
166177
audio_extensions = {".wav", ".flac", ".mp3", ".m4a", ".ogg"}
167178
dir_path = Path(directory)
@@ -179,7 +190,7 @@ def enroll_from_directory(directory: str, speaker_id: str, speaker_name: str) ->
179190
return False
180191

181192
logger.info(f"Found {len(audio_files)} audio files in {directory}")
182-
return enroll_multiple_files(audio_files, speaker_id, speaker_name)
193+
return enroll_multiple_files(audio_files, speaker_id, speaker_name, user_id)
183194

184195

185196
def download_youtube_audio(
@@ -316,6 +327,9 @@ def main():
316327

317328
# Speaker info arguments
318329
parser.add_argument("--id", help="Speaker ID (required for enrollment)")
330+
parser.add_argument(
331+
"--user-id", help="Chronicle user id owning the speaker (required to enroll)"
332+
)
319333
parser.add_argument("--name", help="Speaker display name (required for enrollment)")
320334

321335
# Optional arguments
@@ -347,25 +361,29 @@ def main():
347361

348362
else:
349363
# Enrollment actions require ID and name
350-
if not args.id or not args.name:
351-
logger.error("❌ --id and --name are required for enrollment")
364+
if not args.id or not args.name or not args.user_id:
365+
logger.error("❌ --id, --name and --user-id are required for enrollment")
352366
return 1
353367

354368
if args.file:
355369
success = enroll_single_file(
356-
args.file, args.id, args.name, args.start, args.end
370+
args.file, args.id, args.name, args.user_id, args.start, args.end
357371
)
358372

359373
elif args.files:
360-
success = enroll_multiple_files(args.files, args.id, args.name)
374+
success = enroll_multiple_files(
375+
args.files, args.id, args.name, args.user_id
376+
)
361377

362378
elif args.dir:
363-
success = enroll_from_directory(args.dir, args.id, args.name)
379+
success = enroll_from_directory(args.dir, args.id, args.name, args.user_id)
364380

365381
elif args.youtube:
366382
audio_path = download_youtube_audio(args.youtube, args.start, args.end)
367383
if audio_path:
368-
success = enroll_single_file(audio_path, args.id, args.name)
384+
success = enroll_single_file(
385+
audio_path, args.id, args.name, args.user_id
386+
)
369387
os.unlink(audio_path) # Clean up temporary file
370388
else:
371389
success = False
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
"""Core utilities and shared components."""
22

33
from .utils import (
4-
extract_user_id_from_speaker_id,
54
get_data_directory,
5+
owner_of_speaker,
66
safe_format_confidence,
77
secure_temp_file,
88
validate_confidence,
@@ -12,6 +12,6 @@
1212
"get_data_directory",
1313
"safe_format_confidence",
1414
"secure_temp_file",
15-
"extract_user_id_from_speaker_id",
15+
"owner_of_speaker",
1616
"validate_confidence",
1717
]

extras/speaker-recognition/src/simple_speaker_recognition/api/core/utils.py

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -105,23 +105,19 @@ def secure_temp_file(suffix: str = ".wav") -> tempfile._TemporaryFileWrapper:
105105
return tempfile.NamedTemporaryFile(delete=False, suffix=suffix)
106106

107107

108-
def extract_user_id_from_speaker_id(speaker_id: str) -> str:
109-
"""Return the tenant for ``speaker_id``: its recorded owner, else the id's prefix.
110-
111-
The stored ``speakers.user_id`` is the ownership record and wins whenever the
112-
speaker exists — it stays right for a speaker that was imported, renamed, or
113-
created by a client that formats ids differently, none of which the id convention
114-
survives.
115-
116-
The convention ``user_{user_id}_speaker_{...}`` is only consulted when no such
117-
speaker exists yet, because enrolment *creates* one and the id is currently how
118-
the caller states the tenant. That is a weak contract and the reason the audit
119-
says a tenant should not be inferred from a SpeakerId; removing it needs an
120-
explicit ``user_id`` on /enroll/upload, /enroll/batch and /enroll/append and on
121-
each of their callers (Chronicle's client, the speaker WebUI, and two scripts).
122-
123-
Returns a string either way. It used to return ``int(parts[1])``, which stopped
124-
parsing the moment a tenant became a Chronicle ObjectId.
108+
def owner_of_speaker(speaker_id: str) -> str:
109+
"""Return the tenant that owns ``speaker_id``, from the speaker's own record.
110+
111+
``speakers.user_id`` is the ownership record, so it is the only thing consulted.
112+
This used to parse the tenant out of the id — ``int(speaker_id.split("_")[1])`` —
113+
which the audit flags as inferring a tenant from a SpeakerId: the
114+
``user_{user_id}_speaker_{...}`` shape is a naming convention, not a fact, and it
115+
is wrong for a speaker that was imported or created by a client that names ids
116+
differently. It also stopped parsing entirely once a tenant became a Chronicle
117+
ObjectId rather than an integer.
118+
119+
Only for speakers that already exist. Enrolment creates one, so it takes an
120+
explicit ``user_id`` instead of leaving the service to guess.
125121
"""
126122

127123
# Imported here rather than at module scope: database.queries imports the API
@@ -132,22 +128,12 @@ def extract_user_id_from_speaker_id(speaker_id: str) -> str:
132128
db = get_db_session()
133129
try:
134130
speaker = db.query(Speaker).filter(Speaker.id == speaker_id).first()
135-
if speaker is not None:
136-
return str(speaker.user_id)
131+
if speaker is None:
132+
raise HTTPException(404, f"Speaker not found: {speaker_id}")
133+
return str(speaker.user_id)
137134
finally:
138135
db.close()
139136

140-
if not speaker_id.startswith("user_"):
141-
raise HTTPException(
142-
400,
143-
"Unknown speaker and unrecognised id: expected "
144-
f"'user_{{user_id}}_...', got: {speaker_id}",
145-
)
146-
parts = speaker_id.split("_")
147-
if len(parts) < 3 or not parts[1]:
148-
raise HTTPException(400, f"Cannot determine the owning user from: {speaker_id}")
149-
return parts[1]
150-
151137

152138
def validate_confidence(confidence: Any, context: str = "") -> float:
153139
"""Validate and sanitize confidence values from speaker identification.

0 commit comments

Comments
 (0)