feat(diarize): forward FoxNose speaker turns across the C ABI (#395) - #396
Merged
CrispStrobe merged 1 commit intoAug 28, 2026
Merged
Conversation
Contributor
Author
|
@CrispStrobe I didn't generate the other bindings (Go, Python, Java, Ruby, Dart, JS) yet, as I would prefer to have your review first. |
…trobe#395) Diarization labels the caller's segments, so the label resolution can never be finer than the grid the caller sends in: a segment straddling a speaker change is silently awarded to whoever holds the majority of it, and the minority speaker's words are gone. Sending a finer grid does not help either — FoxNose skips spans under kMinSegmentSeconds = 0.4 s, so a per-word grid starves the embedder and the clusterer under-counts speakers. The information to fix this already existed and was discarded one layer up. crispasr_diarize_segments() has exposed the audio-derived turns since CrispStrobe#324 (out_turns); crispasr_diarize_segments_abi called the C++ overload without that argument, so no Rust / Dart / Go / Python consumer could reach them. Adds crispasr_diarize_segments_turns_abi — a NEW symbol, so the existing ABI stays stable (same append-only convention as crispasr_diarize_opts_abi) — plus crispasr_diarize_turn_abi, the crispasr-sys extern "C" mirror with a layout test, and crispasr::diarize_segments_with_turns in the safe crate. Both entry points share one body; passing NULL / 0 / NULL for the three trailing parameters is exactly the older call. Turns come out in centiseconds on the CALLER's absolute timeline (slice_t0_cs added back), so they compare directly with caller segments without a frame conversion. Truncation follows the crispasr_detect_language_pcm house style: rc 2, with *out_n_turns holding the required capacity. The Rust wrapper sizes the buffer from the audio length (one slot per 0.5 s, above the 0.6 s embedding hop) and retries once, so callers never see it. Additive throughout: the segments are labelled exactly as before, and the methods that derive no turns report 0 rather than an error. Tests, at both levels because only FoxNose derives turns and FoxNose needs a real embedder: - model-free contract in tests/test-session-abi-nulls.cpp — argument validation, "no turn buffer == the older symbol", 0 turns from the methods that derive none; - tests/test-diarize-foxnose-turns-live.cpp for everything needing REAL turns — well-formedness, the truncation protocol, the slice_t0_cs shift, and "asking for turns does not change the labels". Opt-in via CRISPASR_TEST_FOXNOSE_WAV + CRISPASR_TEST_FOXNOSE_EMBEDDER; verified on samples/multispeaker.wav + wespeaker-resnet34-lm, where the fixture does exercise a segment covering two speakers. The same pair of levels on the Rust side in crispasr/tests/integration.rs. Not done, deliberately: Go, Python, Java, Ruby, Dart and JS still expose only crispasr_diarize_segments_abi. Nothing there is broken — the new symbol is additive — but a caller on those surfaces still cannot split a segment. Recorded as a mechanical follow-up in PLAN.md. Closes CrispStrobe#395 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
davideme
force-pushed
the
claude/crisp-asr-issue-395-9d87be
branch
from
August 27, 2026 08:55
11d32cb to
e8104f2
Compare
CrispStrobe
added a commit
that referenced
this pull request
Aug 28, 2026
#396 added crispasr_diarize_segments_turns_abi and bound it from Rust. Go, Java, JavaScript and Ruby all wrap the diarize ABI too, so a Go caller could not reach the turns at all — and the use case (splitting one of your OWN segments that spans a speaker change) is not Rust-specific. Additive, following the shape already in this file: DiarizeSegments kept its signature when DiarizeSegmentsFoxNose arrived for #324, and DiarizeSegmentsWithTurns joins them over the same private helper. The cgo preamble hand-declares crispasr_diarize_turn_abi with the same layout-must-match warning the opts struct already carries, since Go allocates it. Buffer sizing mirrors the Rust wrapper: one slot per 0.5 s of audio (above FoxNose's 0.6 s embedding hop) plus the segment count plus slack, then a single retry when the ABI returns 2 with the capacity it needs. Tested locally: three model-free contract cases (non-FoxNose reports zero turns; asking for turns does not change the labels, which pins the older symbol against edits to the now-shared body; a missing embedder errors rather than crashes) plus a live case gated on FOXNOSE_EMBEDDER + FOXNOSE_WAV, run against the real wespeaker-resnet34-lm on samples/multispeaker.wav — turns come back ordered, forward in time, and densely numbered. Full Go suite still green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Closes #395.
What
Adds
crispasr_diarize_segments_turns_abi— a new symbol, so the current ABI stays stable (the same append-only convention already documented forcrispasr_diarize_opts_abi) — plus thecrispasr_diarize_turn_abiPOD, the matchingcrispasr-sysextern "C"block, and acrispasr::diarize_segments_with_turnswrapper so Rust callers don't hand-roll FFI.Additive plumbing of a value the library already computes. No new model, no algorithm change, no behaviour change for existing callers.
Why
apply_foxnoselabels each caller segment with the turn it overlaps most, so a segment straddling a speaker change is silently awarded to the majority speaker and the minority speaker's words disappear. Callers can't send a finer grid either:core_foxnoseskips spans underkMinSegmentSeconds = 0.4 s, so a per-word grid starves the embedder and the clusterer under-counts speakers.The turns that resolve this already existed —
crispasr_diarize_segments()has exposed them since #324 viaout_turns, and its doc comment describes exactly this use case. The C ABI was the only layer that dropped them, socrispasr-sys, thecrispasrcrate, and every other binding never saw them.Shape
Both entry points share one body;
NULL / 0 / NULLfor the three trailing parameters is exactly the older call.opts->slice_t0_csis added back on the way out), so a turn and a caller segment compare directly with no frame conversion. The library itself works buffer-relative.crispasr_detect_language_pcm's house style: rc2when the buffer was short, with*out_n_turnsholding the required capacity. The segments are still fully labelled in that case and the firstn_turns_capturns are still written.out_turns == NULLwithout_n_turnsset is a pure count query and returns0— nothing can be truncated when no buffer was offered.diarize_segments_with_turnssizes the buffer from the audio length (one slot per 0.5 s, comfortably above the 0.6 s embedding hop) and retries once, so callers never see the truncation protocol. Retry verified by forcing an undersized first buffer.Tests
Two levels, because only FoxNose derives turns and FoxNose needs a real embedder:
tests/test-session-abi-nulls.cpp— model-free contract: argument validation (including a negative cap), "no turn buffer == the older symbol" on identical input, 0 turns from methods that derive none, and the FoxNose load-failure path still returning1without writing a count.tests/test-diarize-foxnose-turns-live.cpp(new) — everything needing real turns: well-formedness (ordered, non-overlapping, inside the audio, always labelled), the truncation protocol end to end, theslice_t0_csshift (same audio shifted by 10 s ⇒ every turn shifted by exactly 10 s), and "asking for turns does not change the labels". Opt-in viaCRISPASR_TEST_FOXNOSE_WAV+CRISPASR_TEST_FOXNOSE_EMBEDDER, skips cleanly without them.crispasr/tests/integration.rs— the same pair of levels on the Rust side.Verified locally on
samples/multispeaker.wav+wespeaker-resnet34-lm.gguf: 110 assertions green, and the fixture does exercise a caller segment covering two speakers (the test asserts that, so it fails loudly if it ever stops being the case rather than passing vacuously).ctest -L unitis green apart from one pre-existing unrelated failure on this branch (every emitted backend name is a name a surface can open, confucius4-tts naming — reproduced with these changes stashed). clang-format, rustfmt and clippy clean on the new code.Deliberately not done
Go, Python, Java, Ruby, Dart and JS still expose only
crispasr_diarize_segments_abi. Nothing there is broken — the new symbol is additive — but a caller on those surfaces still can't split a segment. Each has a hand-written mirror, and the new turn struct is its own 24-byte POD rather than an append to the opts struct, so extending them is mechanical; recorded as a follow-up inPLAN.md.tests/test_binding_parity.py's curated symbol list is unchanged for the same reason — adding the symbol there fails until the Python binding declares it.Happy to reshape the out-array convention or fold in the other bindings if you'd prefer either in this PR.
🤖 Generated with Claude Code