Skip to content

Remove obsolete exclusive diarization API - #32

Merged
praveenperera merged 2 commits into
masterfrom
fix/remove-obsolete-exclusive-api
Sep 16, 2026
Merged

praveenperera merged 2 commits into
masterfrom
fix/remove-obsolete-exclusive-api

Conversation

@praveenperera

@praveenperera praveenperera commented Sep 16, 2026 •

Copy link
Copy Markdown
Member

Summary

  • remove the obsolete binary make_exclusive path that can choose a speaker by cluster index instead of activation score
  • keep DiarizationResult::exclusive_segments as the single supported exclusive output
  • remove two redundant runtime guards found during the final complexity review
  • document the public API additions and removal in the changelog

Verification

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --features "cuda migraphx load-dynamic _metrics" -- -D warnings
  • cargo test --lib

Summary by CodeRabbit

  • New Features

    • Diarization results now support activation-aware exclusive segments for more accurate speaker assignments.
    • Added runtime controls for filterbank session pooling and thread settings.
    • Added support for sharing model sessions when cloning compatible concurrent diarization pipelines.
  • Bug Fixes

    • Improved filterbank session loading for CoreML and CoreMLFast execution modes when split loading is required.
  • Changes

    • Removed the previous discrete diarization exclusivity operation in favor of activation-aware segment generation.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 5fd7185e-b539-4938-b2f7-175c3eedaa1c

📥 Commits

Reviewing files that changed from the base of the PR and between 62e7d98 and 6fc7df4.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • src/inference/embedding/load/sessions.rs
  • src/pipeline/config.rs
  • src/pipeline/types/data.rs
  • src/reconstruct.rs
💤 Files with no reviewable changes (2)
  • src/reconstruct.rs
  • src/pipeline/types/data.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pull request removes obsolete diarization exclusivity helpers, enables filterbank session-pool loading for CoreML split-ORT plans, and simplifies validated thread-count construction without changing its error behavior.

Changes

Runtime and diarization updates

Layer / File(s) Summary
Diarization exclusivity cleanup
src/pipeline/types/data.rs, src/reconstruct.rs, CHANGELOG.md
The obsolete DiscreteDiarization::make_exclusive method and reconstruct::make_exclusive function are removed. The changelog records activation-aware exclusivity through ExclusiveDiarization::from_scored.
Filterbank session-pool loading
src/inference/embedding/load/sessions.rs
The filterbank session pool now loads whenever split ORT loading is enabled, including CoreML and CoreMLFast modes.
Checked thread-count construction
src/pipeline/config.rs
OrtThreadCount::new uses expect after existing zero-count validation. Zero and oversized counts retain their existing errors.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 6fc7d

The reviewed runtime and diarization updates preserve the stated behavior and are ready to merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removal of the obsolete exclusive diarization API.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/remove-obsolete-exclusive-api

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@praveenperera

Copy link
Copy Markdown
Member Author

@greptile-apps please re-review

@greptile-apps

greptile-apps Bot commented Sep 16, 2026 •

Copy link
Copy Markdown
Greptile Summary

The PR removes the obsolete binary exclusive-diarization API, retains activation-aware exclusive segment generation, simplifies validated runtime configuration and session-loading paths, and documents the public API changes. The post-review update correctly clarifies that OwnedDiarizationPipeline::clone_shared is available only for non-CoreML builds.

Confidence Score: 5/5

The PR appears safe to merge; the previous changelog concern is resolved and no new actionable issues were identified.

The changelog now matches the #[cfg(not(feature = "coreml"))] availability of clone_shared, and the latest change introduces no new correctness or compatibility issue.

Important Files Changed
Filename Overview
CHANGELOG.md Documents the exclusive-diarization API removal and accurately qualifies clone_shared availability.
src/inference/embedding/load/sessions.rs Removes a redundant CoreML condition from split filterbank pool loading.
src/pipeline/config.rs Simplifies construction of a nonzero thread count after existing bounds validation.
src/pipeline/types/data.rs Removes the obsolete public DiscreteDiarization::make_exclusive method.
src/reconstruct.rs Removes the corresponding obsolete binary exclusivity helper.

Reviews (2): Last reviewed commit: "Clarify shared pipeline availability" | Re-trigger Greptile

Comment thread CHANGELOG.md Outdated
@praveenperera

Copy link
Copy Markdown
Member Author

@greptile-apps please re-review

@praveenperera
praveenperera merged commit cd7ea8e into master Sep 16, 2026
14 checks passed
@praveenperera
praveenperera deleted the fix/remove-obsolete-exclusive-api branch September 16, 2026 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant