feat(dub): optional speaker-count hint for diarization (#274) - #275
Conversation
When a clip has multiple speakers, pyannote's auto-detect sometimes collapses
them into a single "Speaker 1" — so the transcript merges turns and the dub
mixes voices. The diarization-consumption side is correct (overlap-weighted,
distinct Speaker N ids — pinned by a new test), so the collapse comes from
auto-detect itself.
Add an optional speaker-count hint (the reporter's own suggestion):
- backend: `/dub/transcribe-stream/{job_id}?num_speakers=N` (clamped 1–20;
None → auto-detect) threaded to `diar_pipe(audio, num_speakers=N)`. Omitted
entirely when unset so we don't depend on the kwarg in every pyannote build.
- frontend: `dubNumSpeakers` store field + a compact "Speakers" number input
in the dub panel (placeholder "Auto") + i18n; `transcribeStreamUrl` appends
the param; the SSE hook reads the hint at stream-open time.
Tests: tests/test_assign_speakers_from_diarization.py (multi-speaker split,
overlap weighting, label robustness, empty-result safety) +
dub.transcribeUrl.test.ts (param appended only for a positive int). Full
backend diarization + frontend suites pass; CJK i18n guard passes.
Does NOT close #274 — pending the reporter confirming that setting the count
resolves the collapse on their video (can't verify pyannote behaviour without
a CUDA box + the clip).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR adds optional speaker-count hinting for dub transcription diarization. The backend endpoint accepts a clamped ChangesSpeaker count hint feature and diarization tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| backend/api/routers/dub_core.py | Adds num_speakers query param to the transcribe-stream endpoint; clamped to [1-20] with a redundant try/except (FastAPI already coerces Optional[int]), and the diarization call site uses a truthiness check instead of is not None. |
| frontend/src/hooks/useDubWorkflow.js | Reads dubNumSpeakers from the Zustand store via getState() at SSE open-time inside _waitForTranscribe; correctly uses the imperative accessor so all three call sites pick up the user's current choice without dependency-array changes. |
| frontend/src/api/dub.ts | Extends transcribeStreamUrl to accept an optional numSpeakers hint; appends the floored integer as a query param only when positive and finite. |
| frontend/src/store/dubSlice.ts | Adds dubNumSpeakers: number |
| frontend/src/pages/DubTab.jsx | Adds a compact number input (min 1, max 20, placeholder Auto) wired to dubNumSpeakers; disabled during upload/transcribe steps; onChange correctly nullifies out-of-range values. |
| frontend/src/api/dub.transcribeUrl.test.ts | New unit tests cover omit-when-absent, omit-for-null/0/negative/NaN, append-for-positive-int, and floor-fractional behaviours. |
| tests/test_assign_speakers_from_diarization.py | New backend test suite covering multi-speaker assignment, overlap weighting, midpoint fallback, non-underscore label passthrough, and empty-diarization safety. |
| frontend/src/i18n/locales/en.json | Adds three i18n keys for the speaker-count input: label, placeholder, and tooltip help text. |
| frontend/src/pages/DubTab.css | Adds two CSS classes for the speakers hint container and its number input; uses CSS custom properties for theming. |
Sequence Diagram
sequenceDiagram
participant User
participant DubTab
participant dubSlice
participant useDubWorkflow
participant dubAPI
participant Backend
participant pyannote
User->>DubTab: Enter num_speakers (or leave blank)
DubTab->>dubSlice: "setDubNumSpeakers(N | null)"
User->>DubTab: Click Upload and Transcribe
DubTab->>useDubWorkflow: handleDubUpload()
useDubWorkflow->>dubSlice: getState().dubNumSpeakers
dubSlice-->>useDubWorkflow: "N | null"
useDubWorkflow->>dubAPI: transcribeStreamUrl(jobId, N)
dubAPI-->>useDubWorkflow: "URL with ?num_speakers=N if N > 0"
useDubWorkflow->>Backend: "EventSource /dub/transcribe-stream/job_id?num_speakers=N"
Backend->>Backend: Clamp N to 1-20 or None
alt num_speakers provided
Backend->>pyannote: "diar_pipe(audio, num_speakers=N)"
else auto-detect
Backend->>pyannote: diar_pipe(audio)
end
pyannote-->>Backend: Annotation
Backend->>Backend: assign_speakers_from_diarization(segments, diar)
Backend-->>useDubWorkflow: SSE final event with diarized segments
Reviews (1): Last reviewed commit: "feat(dub): optional speaker-count hint f..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if num_speakers is not None: | ||
| try: | ||
| num_speakers = int(num_speakers) | ||
| num_speakers = num_speakers if 1 <= num_speakers <= 20 else None | ||
| except (TypeError, ValueError): | ||
| num_speakers = None |
There was a problem hiding this comment.
Optional[int]
Because the parameter is declared as Optional[int], FastAPI validates and converts the query string before the function body runs. If a caller passes ?num_speakers=abc, FastAPI returns a 422 Unprocessable Entity before this code is ever reached. The int() cast is therefore a no-op on an already-int value, and the (TypeError, ValueError) handler is unreachable. Only the range clamp (1 ≤ n ≤ 20) does real work here — the try/except wrapper around it is misleading dead code.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if num_speakers: | ||
| logger.info("Diarizing with num_speakers=%d (user hint)", num_speakers) | ||
| diar = diar_pipe(asr_audio_target, num_speakers=num_speakers) | ||
| else: | ||
| diar = diar_pipe(asr_audio_target) |
There was a problem hiding this comment.
if num_speakers:) instead of if num_speakers is not None: is correct only because the clamping block above guarantees the value is either in [1, 20] or None — 0 is never reachable here. However, is not None makes the intent explicit and is the conventional Python idiom for optional sentinel checks, making the code easier to audit at a glance.
| if num_speakers: | |
| logger.info("Diarizing with num_speakers=%d (user hint)", num_speakers) | |
| diar = diar_pipe(asr_audio_target, num_speakers=num_speakers) | |
| else: | |
| diar = diar_pipe(asr_audio_target) | |
| if num_speakers is not None: | |
| logger.info("Diarizing with num_speakers=%d (user hint)", num_speakers) | |
| diar = diar_pipe(asr_audio_target, num_speakers=num_speakers) | |
| else: | |
| diar = diar_pipe(asr_audio_target) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_assign_speakers_from_diarization.py (1)
34-81: 💤 Low valueOptional: Replace en-dashes with hyphens in comments.
Static analysis (RUF003) flags several en-dash characters (–) in comments where hyphens (-) are expected. This is a cosmetic linter compliance issue with no functional impact.
Examples flagged
- Line 35:
0–5s→0-5s- Line 48:
2–8→2-8,2–5→2-5- Line 56:
4–5→4-5,5–11→5-11🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_assign_speakers_from_diarization.py` around lines 34 - 81, Replace the cosmetic en-dash characters (–) used in comments with standard hyphens (-) to satisfy linter RUF003; update the comment texts inside the test functions (e.g., test_two_speakers_yield_two_distinct_ids, test_overlap_weighted_winner, test_midpoint_fallback_when_no_overlap) so phrases like "0–5s", "2–8", "2–5", "4–5", and "5–11" use hyphens ("0-5s", "2-8", "2-5", "4-5", "5-11") instead, leaving all code and assertions unchanged.backend/api/routers/dub_core.py (1)
692-696: 💤 Low valueConsider using explicit None check for clarity.
The truthiness check
if num_speakers:is functionally correct (validation ensures it's eitherNoneor1–20), butif num_speakers is not None:would make the intent more explicit and robust against future changes.♻️ Proposed refactor
- if num_speakers: + if num_speakers is not None: logger.info("Diarizing with num_speakers=%d (user hint)", num_speakers) diar = diar_pipe(asr_audio_target, num_speakers=num_speakers) else:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/api/routers/dub_core.py` around lines 692 - 696, The truthy check on num_speakers should be made explicit: replace the conditional `if num_speakers:` with `if num_speakers is not None:` so the intent is clear and future falsy-but-valid values won't break behavior; ensure you keep the existing logger.info("Diarizing with num_speakers=%d (user hint)", num_speakers) and the call to diar_pipe(asr_audio_target, num_speakers=num_speakers) in that branch, and call diar_pipe(asr_audio_target) in the else branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/api/routers/dub_core.py`:
- Around line 692-696: The truthy check on num_speakers should be made explicit:
replace the conditional `if num_speakers:` with `if num_speakers is not None:`
so the intent is clear and future falsy-but-valid values won't break behavior;
ensure you keep the existing logger.info("Diarizing with num_speakers=%d (user
hint)", num_speakers) and the call to diar_pipe(asr_audio_target,
num_speakers=num_speakers) in that branch, and call diar_pipe(asr_audio_target)
in the else branch.
In `@tests/test_assign_speakers_from_diarization.py`:
- Around line 34-81: Replace the cosmetic en-dash characters (–) used in
comments with standard hyphens (-) to satisfy linter RUF003; update the comment
texts inside the test functions (e.g., test_two_speakers_yield_two_distinct_ids,
test_overlap_weighted_winner, test_midpoint_fallback_when_no_overlap) so phrases
like "0–5s", "2–8", "2–5", "4–5", and "5–11" use hyphens ("0-5s", "2-8", "2-5",
"4-5", "5-11") instead, leaving all code and assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 943d1993-f7a7-4d3c-9d2c-d6e4d61a5447
📒 Files selected for processing (9)
backend/api/routers/dub_core.pyfrontend/src/api/dub.transcribeUrl.test.tsfrontend/src/api/dub.tsfrontend/src/hooks/useDubWorkflow.jsfrontend/src/i18n/locales/en.jsonfrontend/src/pages/DubTab.cssfrontend/src/pages/DubTab.jsxfrontend/src/store/dubSlice.tstests/test_assign_speakers_from_diarization.py
1 participant
Problem
Issue #274 — with multiple speakers in a video, OmniVoice detects only one ("Speaker 1"): the transcript merges everyone's lines and the dub mixes voices. Reporter is on v0.3.5 with pyannote diarization now loading (after #270).
I traced the full path. The consumption side is correct —
assign_speakers_from_diarizationoverlap-weights each segment and emits distinctSpeaker Nids (pinned by a new unit test). So the collapse comes from pyannote's auto-detect returning a single speaker for this audio — which is exactly what an explicit count hint fixes (and what the reporter asked for).Change
Optional speaker-count hint, end to end:
/dub/transcribe-stream/{job_id}?num_speakers=N(clamped 1–20;None→ auto-detect) threaded todiar_pipe(audio, num_speakers=N). Omitted entirely when unset, so we don't depend on the kwarg existing in every pyannote build.dubNumSpeakersstore field + a compact "Speakers" number input (placeholder Auto) in the dub panel + i18n keys;transcribeStreamUrlappends the param; the SSE hook reads the hint at stream-open time.Tests
tests/test_assign_speakers_from_diarization.py— multi-speaker split, overlap weighting, non-underscore label kept verbatim, empty-result safety.dub.transcribeUrl.test.ts— param appended only for a positive int (floored), omitted for null/0/neg/NaN.typecheck:ci✅; build ✅; CJK i18n guard ✅.Scope / honesty
Default behavior unchanged (blank = auto-detect, exactly as today). I can't run pyannote on the reporter's clip without a CUDA box, so this is the verifiable plumbing + the control they requested — #274 stays open until they confirm setting the count resolves it. If pyannote still under-segments with an explicit count, that's a model-level follow-up.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests