fix(diarization): use_auth_token->token shim for pyannote on HF Hub 1.x (#167) - #168
Conversation
….x (#167) pyannote-audio 3.x (pipeline.py:102) calls hf_hub_download(use_auth_token=...), which huggingface_hub 1.x removed (only 'token' now) -> 'unexpected keyword argument use_auth_token', breaking speaker diarization. Wrap hf_hub_download/ snapshot_download to translate the dead kwarg, applied before pyannote's 'from huggingface_hub import hf_hub_download' binds it (+ patch already-loaded pyannote modules). Verified the real pyannote reference binds the shim. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds an idempotent compatibility shim that wraps ChangesPyannote Hugging Face Hub Compatibility for Pyannote
sequenceDiagram
participant Caller as get_diarization_pipeline
participant Shim as _ensure_pyannote_hf_token_compat
participant HF as huggingface_hub.hf_hub_download
participant Pyannote as pyannote.audio.core.pipeline
Caller->>Shim: install shim before importing Pipeline
Shim->>HF: wrap hf_hub_download/snapshot_download (translate kwargs)
Shim->>Pyannote: patch already-imported bindings to use shim
Caller->>Pyannote: import/bind Pipeline (uses shim-wrapped HF functions)
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…d-local) CodeQL doesn't model pytest.skip() as no-return, so it flagged _pp as a possibly-uninitialized local (py/uninitialized-local-variable, error). Switch to pytest.importorskip — cleaner and CodeQL-clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| backend/services/model_manager.py | Adds _ensure_pyannote_hf_token_compat() shim that wraps hf_hub_download/snapshot_download to translate use_auth_token to token; called in get_diarization_pipeline before pyannote import. The setdefault translation has a silent-drop edge case if token=None is passed explicitly alongside use_auth_token. |
| tests/test_pyannote_hf_compat.py | Three tests covering kwarg translation, idempotency, and a live pyannote-binding check. test_pyannote_binds_the_shim permanently patches the real hf_hub_download without monkeypatch cleanup, and the token=None+use_auth_token=X edge case is not covered. |
Sequence Diagram
sequenceDiagram
participant App as get_diarization_pipeline()
participant Shim as _ensure_pyannote_hf_token_compat()
participant HF as huggingface_hub module
participant PyA as pyannote.audio.Pipeline
participant DL as hf_hub_download (real)
App->>Shim: call (before pyannote import)
Shim->>HF: setattr(hf_hub_download, wrapped_fn)
Note over HF: wrapped_fn translates use_auth_token to token
App->>PyA: from pyannote.audio import Pipeline
Note over PyA: binds wrapped hf_hub_download
App->>PyA: "Pipeline.from_pretrained(..., use_auth_token=token)"
PyA->>HF: "hf_hub_download(..., use_auth_token=token)"
Note over HF: shim intercepts and translates kwarg
HF->>DL: "hf_hub_download(..., token=token)"
DL-->>App: model downloaded
Reviews (1): Last reviewed commit: "test(diarization): use pytest.importorsk..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if "use_auth_token" in kwargs: | ||
| kwargs.setdefault("token", kwargs.pop("use_auth_token")) |
There was a problem hiding this comment.
setdefault call silently discards use_auth_token when token=None is already present in kwargs. If any caller (or a future pyannote minor that starts passing token=None as an explicit default) issues hf_hub_download(..., token=None, use_auth_token=real_token), the real token is popped and thrown away — the wrapped function receives token=None and authentication fails with no error pointing at the shim.
| if "use_auth_token" in kwargs: | |
| kwargs.setdefault("token", kwargs.pop("use_auth_token")) | |
| if "use_auth_token" in kwargs: | |
| uat = kwargs.pop("use_auth_token") | |
| if not kwargs.get("token"): | |
| kwargs["token"] = uat |
| _ensure_pyannote_hf_token_compat() # #167: use_auth_token -> token | ||
| from pyannote.audio import Pipeline | ||
| logger.info("Loading Pyannote Diarization Pipeline...") | ||
| _diar_pipeline = Pipeline.from_pretrained("pyannote/speaker-diarization-3.1", use_auth_token=hf_token) |
There was a problem hiding this comment.
Pipeline.from_pretrained call still passes the deprecated use_auth_token kwarg. pyannote 3.x still accepts it, so this doesn't break today, but it's inconsistent with the fix this PR applies — and if a pyannote upgrade is ever attempted, the outer call will be the first place to notice. Switching to token= here is a one-line change and removes the last use_auth_token site from the call path.
| _diar_pipeline = Pipeline.from_pretrained("pyannote/speaker-diarization-3.1", use_auth_token=hf_token) | |
| _diar_pipeline = Pipeline.from_pretrained("pyannote/speaker-diarization-3.1", token=hf_token) |
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!
| def test_pyannote_binds_the_shim(): | ||
| """The real proof: after the shim, pyannote's own `hf_hub_download` | ||
| reference translates `use_auth_token` rather than raising.""" | ||
| _ensure_pyannote_hf_token_compat() | ||
| # importorskip imports the module (or skips) — and since the shim patched | ||
| # huggingface_hub first, pyannote's `from huggingface_hub import | ||
| # hf_hub_download` (pipeline.py:34) binds the wrapped fn. (Also avoids the | ||
| # CodeQL "possibly-uninitialized local" false positive from a try/skip.) | ||
| _pp = pytest.importorskip("pyannote.audio.core.pipeline") | ||
| assert getattr(_pp.hf_hub_download, "_ov_uat_shim", False), ( | ||
| "pyannote.audio.core.pipeline.hf_hub_download is not the use_auth_token shim" | ||
| ) |
There was a problem hiding this comment.
test_pyannote_binds_the_shim calls _ensure_pyannote_hf_token_compat() against the real huggingface_hub without a monkeypatch fixture, so the production hf_hub_download reference is permanently wrapped for the remainder of the test session. The shim is transparent and idempotent so this is benign today, but any later test that explicitly checks huggingface_hub.hf_hub_download is the unwrapped original would spuriously fail. Wrapping the call in a monkeypatch (or simply accepting the permanent patch as intentional with a comment) would make the side-effect explicit.
2 participants
Fixes #167. pyannote-audio 3.x (
core/pipeline.py:102) callshf_hub_download(..., use_auth_token=...); huggingface_hub 1.x removed that kwarg (onlytoken), so diarization died withunexpected keyword argument 'use_auth_token'. We can't drop to HF Hub <1 (transformers≥5.3 + our token persistence need 1.x) nor jump to pyannote 4.x (whisperx 3.4.2 still passesuse_auth_tokento pyannote'sInference— already noted in pyproject). So: a small shim wrapshf_hub_download/snapshot_downloadto translateuse_auth_token→token, applied before pyannote'sfrom huggingface_hub import …binds it. Tested with the installed pyannote 3.4.0 + HF Hub 1.7.2 — incl. a real check that pyannote's bound reference is the shim. 3 tests.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests