Conversation
…ive-window clamp DiarStream::speaker_for_frames() clamped any query range predating the retained live probability window (probs_base_) to the window's first frame, on the documented assumption that word tagging always happens "near the frontier" (i.e. shortly after the audio is processed). That assumption holds for streaming/interactive use, where next() is called throughout, but not for a whole-file batch pass: Recognizer::recognize() (used by `transcribe --diarize`) does exactly one push() of the entire file followed by one finish(), tagging every word in a single pass at the very end. For any file long enough to trigger DiarStream's timeline compaction (~20 min by default), every word before the last ~10-20 minutes was silently misattributed to whatever speaker sat at the live window's edge instead of its true historical speaker - collapsing combined-mode diarization onto one dominant identity for most of a long recording, while the standalone `diarize` command (whose segments() already folds frozen_segs_ back in) was unaffected. Fix: extract the frame-range resolution into speaker_for_frame_range() and, when a query starts before probs_base_, resolve it against the frozen segments frozen_segs_ already retains for that span (nearest segment by time if the range falls in a gap), instead of clamping into the live window. Verified against a real 2h39m two-speaker interview that previously collapsed onto one speaker for ~140 of 160 minutes: with this fix, both speakers now alternate proportionally across every 10-minute window of the full recording, matching the standalone diarize command's (unaffected) output on the same audio. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VksfgLsiSbWfkmyRTTiPww
fix(diar): resolve compacted word ranges via frozen segments, not a live-window clamp
Tracks where this fork's main has diverged from NVIDIA/NeMo-Speech.cpp, so a future upstream rebase/merge knows what to reconcile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VksfgLsiSbWfkmyRTTiPww
Diarization results currently only reach the client at commit time, so NeMoSpeech's SilenceCommitTrigger (its only segmentation signal) can go up to 15s without finalizing during continuous speech. DiarStream already tracks confirmed speaker segments incrementally (segments(), valid mid-stream) -- this spec adds a new, additive WS event that surfaces a segment's speaker change as soon as it's confirmed, so a client can commit immediately instead of waiting on silence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
…tion Implements pure speaker-change detection logic: compares the most recent confirmed segment's speaker against last_reported and returns the new speaker + that segment's start time if they differ. Segments must be sorted ascending by t0 (guaranteed by DiarStream::segments()). Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
Wraps Task 1's detect_speaker_change() pure function as a stateful per-stream poll method, tracking last_reported_speaker_ across calls. Safe to call any time; returns nullopt when diarization isn't enabled. Extends test_diar_recognizer.cpp to poll after each push and require at least one detected change when min_speakers > 1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
Wire RecognitionStream::poll_speaker_change() into the realtime WS handler's append_audio lambda: after draining ASR results for a pushed chunk, poll for a confirmed speaker change and emit conversation.item.speaker_diarization.changed with the 1-based speaker id (wire convention) and start_time. This is the only place in the feature that converts the 0-based DiarSpeakerChange::speaker to the wire's 1-based convention. Extends the WebSocket section of the HTTP conformance test to assert the event fires (when --diar-model is set) with a well-formed, 1-based speaker id and a start_time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
Adds a FORK_CHANGES.md entry documenting the 2026-09-13 live speaker-change event feature, following the format of the existing 2026-09-10 entry. Documents the upstream base commit, affected files, feature description, and verification. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
Single fix-and-close round for feat/live-speaker-change-event's whole-branch review (see .superpowers/sdd/2026-09-13-live-speaker-change-event/progress.md for the full ledger and rulings behind each item), not a new feature task. Critical: RecognitionStream::poll_speaker_change() fired on every stream's very first confirmed segment (nullopt != any_speaker), and since http_server.cpp recreates the stream on every input_audio_buffer.commit, this produced a self-sustaining commit loop every ~1.6-2.4s during continuous single-speaker speech. Fixed by adding SpeakerChangeTracker (src/asr/diar/diar_pipeline.h), which wraps the unchanged, already-tested detect_speaker_change() with a "don't announce the first observation" policy. RecognitionStream now owns one instead of a raw last_reported optional. Added 5 new unit tests to test_diar_speaker_change.cpp (10 total) and corrected the spec's Server implementation section, which had described the old buggy algorithm and contradicted its own Testing section. Also: documented the segments().back() overlapping-speech limitation (code comment + spec "Known limitations" section) rather than redesigning around speaker_for_frames() in this round; reformatted http_conformance_test.py with black and merged its two consecutive `if args.diar_model:` blocks; added the new event type to docs/api.md; corrected FORK_CHANGES.md's overstated "verified against a real multi-speaker fixture" claim (no such fixture exists in either repo) and its missing docs/superpowers/ file entries; switched diar_segments_from_probs to stable_sort for deterministic tie-breaking on identical t0; added a spec note that start_time/speaker are scoped to the current stream; and commented the deliberate non-goal of not polling for a speaker change after finish(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YJ2yUjkW5YwZX6WhNAeSWk
bsips
requested review from
anand-nv,
pskrunner14 and
rmittal-github
as code owners
September 13, 2026 23:45
for more information, see https://pre-commit.ci
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe change fixes diarization lookup for compacted audio ranges and adds realtime speaker-change events. Recognition streams track confirmed transitions, the HTTP server emits them, and tests and documentation cover the new behavior. ChangesDiarization and realtime speaker events
Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant WebSocketClient
participant HTTPRealtimeHandler
participant RecognitionStream
participant SpeakerChangeTracker
WebSocketClient->>HTTPRealtimeHandler: send streamed audio
HTTPRealtimeHandler->>RecognitionStream: process audio
RecognitionStream->>SpeakerChangeTracker: poll speaker change
SpeakerChangeTracker-->>RecognitionStream: return confirmed transition
RecognitionStream-->>HTTPRealtimeHandler: return speaker and start time
HTTPRealtimeHandler-->>WebSocketClient: emit speaker_diarization.changed
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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
conversation.item.speaker_diarization.changedWebSocket event, built onDiarStream::segments()'s existing incremental, hysteresis-cleaned segment tracking.SpeakerChangeTrackerthat swallows the first observation per stream.Test plan
ctest --test-dir build/cpu-asr --output-on-failure— 10/10 passingtest_diar_recognizerrun against a real ASR + diarizer model end-to-endtests/integration/http_conformance_test.pyWebSocket section, including new speaker-change assertionsspeakerconversion on the actual wireblack --checkcleanKnown limitation (documented in the spec): the event compares against the diarizer's most-recently-started confirmed segment, which can misattribute the active speaker during genuinely overlapping/interjecting speech until a future fix using frontier-aware lookup.
See
docs/superpowers/specs/2026-09-13-live-speaker-change-event-design.mdfor the full design.Summary by CodeRabbit
New Features
Bug Fixes
Documentation