From f1e035aebc9fb18221d3a91a57a9db4dc8758bd7 Mon Sep 17 00:00:00 2001 From: Schaka <2223171+Schaka@users.noreply.github.com> Date: Tue, 21 Jul 2026 22:08:18 +0200 Subject: [PATCH 1/5] plugin: let plugins work inside analysis, not just after it Adds two provider-agnostic seams so an accelerator (GPU, NPU, ...) can be shipped as an out-of-tree plugin instead of forking core: * register_onnx_provider() gains only_models/exclude_models so a plugin can offer an ONNX execution provider for just the graphs it can compile, scoped by the session label passed to create_onnx_session (musicnn, clap, whisper_encoder, ...). resolve_providers() honors the scope and leaves unmatched models on their default chain. * register_analysis_provider(component, factory) is wired end to end (it was a logging-only stub). A plugin can replace a whole analysis component; core consults the registered provider first and falls back to the built-in. The lyrics pipeline resolves its ASR/Whisper backend through lyrics/_asr_backend so the built-in ONNX Whisper can be swapped without touching call sites. clap_analyzer pins the CLAP audio model's symbolic time axis to a static shape for any provider whose graph compiler needs it, generalizing the existing CoreML-only path. --- docs/PLUGIN.md | 6 +- lyrics/__init__.py | 16 +- lyrics/_asr_backend.py | 42 +++++ lyrics/lyrics_transcriber.py | 8 +- plugin/api.py | 33 +++- plugin/manager.py | 21 +++ tasks/analysis/song.py | 23 ++- tasks/clap_analyzer.py | 22 ++- test/unit/test_plugin_analysis_seams.py | 237 ++++++++++++++++++++++++ 9 files changed, 375 insertions(+), 33 deletions(-) create mode 100644 lyrics/_asr_backend.py create mode 100644 test/unit/test_plugin_analysis_seams.py diff --git a/docs/PLUGIN.md b/docs/PLUGIN.md index a97014bb5..1df0c2060 100644 --- a/docs/PLUGIN.md +++ b/docs/PLUGIN.md @@ -24,7 +24,8 @@ Everything a plugin can do, at a glance. The last column tells you where to find | 12 | Named worker tasks | `add_task` | No - see "API reference" | | 13 | Startup hooks | `on_flask_start` / `on_worker_start` | No - see "The plugin lifecycle" | | 14 | Create media-server playlists | `tasks.mediaserver` | No - see "Create a playlist on your media server" | -| 15 | Extra ONNX execution provider | `register_onnx_provider` | No - see "API reference" (advanced) | +| 15 | Extra ONNX execution provider (optionally scoped per model) | `register_onnx_provider` | No - see "API reference" (advanced) | +| 16 | Replace an analysis component (e.g. the ASR/Whisper backend) | `register_analysis_provider` | No - see "API reference" (advanced) | ## Installing and managing plugins @@ -468,7 +469,8 @@ And the methods on the `ctx` object in `register(ctx)`: | `add_task(name, func, queue='default')` | Register a named worker task. It appears on the Scheduled Tasks page like a cron task, so the admin can schedule it or run it now. | | `on_install(func)` | Run once at install and on every update. Gets the database connection. | | `on_flask_start(func)` / `on_worker_start(func)` | Run at every start of that container, after the plugin loads. | -| `register_onnx_provider(name, options, position)` | Advanced: offer an extra ONNX Runtime execution provider (for example a GPU) for analysis. | +| `register_onnx_provider(name, options, position, only_models=None, exclude_models=None)` | Advanced: offer an extra ONNX Runtime execution provider (for example a GPU) for analysis. Scope it to specific models with `only_models`/`exclude_models` (lists of the session `label`: `musicnn`, `clap`, `whisper_encoder`, ...) when the provider can only parse some graphs. | +| `register_analysis_provider(component, factory)` | Advanced: replace a whole analysis component with a plugin-supplied implementation. `component` is currently `asr` (the Whisper backend); `factory` is the replacement module/object (or a zero-arg callable returning one) matching the built-in surface `load_whisper_model`/`transcribe`/`is_loaded`/`unload`. Consulted before the built-in. | ## Who can see and manage plugins diff --git a/lyrics/__init__.py b/lyrics/__init__.py index b10c119f0..cf78da9eb 100644 --- a/lyrics/__init__.py +++ b/lyrics/__init__.py @@ -82,10 +82,13 @@ def is_lyrics_loaded() -> bool: if not _LYRICS_ENABLED: return False try: - from . import whisper_onnx, gte_onnx, silero_onnx + from ._asr_backend import get_asr_backend + from . import gte_onnx, silero_onnx + + whisper_mod = get_asr_backend() except Exception: return False - for mod in (whisper_onnx, gte_onnx, silero_onnx): + for mod in (whisper_mod, gte_onnx, silero_onnx): try: if mod.is_loaded(): return True @@ -100,12 +103,13 @@ def unload_lyrics_models() -> bool: released_any = False try: try: - from . import whisper_onnx + from ._asr_backend import get_asr_backend - if whisper_onnx.is_loaded(): - released_any = bool(_safe_call('whisper_onnx.unload', whisper_onnx.unload)) + whisper_mod = get_asr_backend() + if whisper_mod.is_loaded(): + released_any = bool(_safe_call('whisper.unload', whisper_mod.unload)) except Exception as exc: - _logger.warning("Lyrics whisper_onnx release failed: %s", exc) + _logger.warning("Lyrics whisper release failed: %s", exc) try: from . import gte_onnx diff --git a/lyrics/_asr_backend.py b/lyrics/_asr_backend.py new file mode 100644 index 000000000..2e28dc410 --- /dev/null +++ b/lyrics/_asr_backend.py @@ -0,0 +1,42 @@ +# AudioMuse-AI - https://github.com/NeptuneHub/AudioMuse-AI +# Copyright (C) 2025 NeptuneHub +# SPDX-License-Identifier: AGPL-3.0-only +# +# This program is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License v3.0. See the LICENSE file +# in the project root or + +"""Resolves the Whisper/ASR backend for the lyrics pipeline. + +The default is the built-in ONNX backend (whisper_onnx). A plugin can replace it +by registering an alternative with ``ctx.register_analysis_provider('asr', ...)`` +- used, for example, by an AMD/ROCm plugin that swaps in faster-whisper because +MIGraphX cannot run the ONNX Whisper decoder. The replacement must expose the +same public surface: ``load_whisper_model()``, ``transcribe(wav, sr, language=None)``, +``is_loaded()`` and ``unload()``. + +Main Features: +* Consults the loaded plugins for an 'asr' analysis provider before the built-in +* Falls back to the built-in whisper_onnx backend when no plugin registered one +* Swallows any plugin-resolution error so a broken plugin never breaks lyrics +""" + +from __future__ import annotations + + +def get_asr_backend(): + override = _plugin_asr_backend() + if override is not None: + return override + from . import whisper_onnx + + return whisper_onnx + + +def _plugin_asr_backend(): + try: + from plugin.manager import plugin_manager + + return plugin_manager.get_analysis_provider('asr') + except Exception: + return None diff --git a/lyrics/lyrics_transcriber.py b/lyrics/lyrics_transcriber.py index 312f6298f..25b9d756b 100644 --- a/lyrics/lyrics_transcriber.py +++ b/lyrics/lyrics_transcriber.py @@ -218,9 +218,9 @@ def _apply_thread_env(num_threads: int) -> None: def load_asr_model(num_threads: Optional[int] = None): threads = num_threads or get_lyrics_threads() _apply_thread_env(threads) - from .whisper_onnx import load_whisper_model as _load + from ._asr_backend import get_asr_backend - return _load() + return get_asr_backend().load_whisper_model() def load_topic_embedding_model(model_name: Optional[str] = None): @@ -704,9 +704,9 @@ def _transcribe( ) -> Dict[str, object]: if audio is None or len(audio) == 0: return {'text': '', 'language': language or '', 'duration': 0.0} - from .whisper_onnx import transcribe as _whisper_transcribe + from ._asr_backend import get_asr_backend - return _whisper_transcribe(audio, sr, language=language) + return get_asr_backend().transcribe(audio, sr, language=language) def _embed_text(text: str, tokenizer, model) -> Optional[np.ndarray]: diff --git a/plugin/api.py b/plugin/api.py index 9752ee7f4..b887e7e4c 100644 --- a/plugin/api.py +++ b/plugin/api.py @@ -200,6 +200,7 @@ def __init__(self, plugin_id, role): self.cron_tasks = {} self.tasks = {} self.onnx_providers = [] + self.analysis_providers = {} self.flask_start = [] self.worker_start = [] self.song_analyzed_hooks = [] @@ -225,8 +226,15 @@ def add_task(self, name, func, queue='default'): def add_cron_task(self, name, func, queue='default'): self.cron_tasks[name] = {'dotted': dotted_path(func), 'queue': queue} - def register_onnx_provider(self, name, options=None, position='before_cpu'): - self.onnx_providers.append({'name': name, 'options': options or {}, 'position': position}) + def register_onnx_provider(self, name, options=None, position='before_cpu', + only_models=None, exclude_models=None): + self.onnx_providers.append({ + 'name': name, + 'options': options or {}, + 'position': position, + 'only_models': list(only_models) if only_models else None, + 'exclude_models': list(exclude_models) if exclude_models else None, + }) def on_flask_start(self, func): self.flask_start.append(func) @@ -249,9 +257,18 @@ def on_song_analyzed(self, func): def on_install(self, func): self.install_hooks.append(func) - def register_analysis_provider(self, *args, **kwargs): - logger.info( - 'Plugin %s called register_analysis_provider; alternative analysis models ' - 'are a forward seam and are not wired in this version.', - self.plugin_id, - ) + def register_analysis_provider(self, component, factory): + """Replace a whole analysis component with a plugin-supplied implementation. + + Some accelerators need more than a different ONNX execution provider: they + need a different library entirely. MIGraphX, for instance, cannot run the + ONNX Whisper decoder at all, so an AMD plugin swaps in faster-whisper. + + ``component`` names the step to replace (currently ``asr``). ``factory`` is + the replacement module/object, or a zero-arg callable returning one. It must + match the built-in module's public surface; for ``asr`` that is + ``load_whisper_model()``, ``transcribe(wav, sr, language=None)``, + ``is_loaded()`` and ``unload()``. Core consults the registered provider + first and falls back to the built-in when no plugin registered one. + """ + self.analysis_providers[component] = factory diff --git a/plugin/manager.py b/plugin/manager.py index febcc05d6..60ea188f8 100644 --- a/plugin/manager.py +++ b/plugin/manager.py @@ -324,6 +324,7 @@ def sync(self, conn=None, role=None): record['settings_endpoint'] = None record['cron_tasks'] = {} record['onnx_providers'] = [] + record['analysis_providers'] = {} record['song_analyzed_hooks'] = [] record['error'] = None records[row['id']] = record @@ -600,6 +601,7 @@ def load(self, role, flask_app=None): record['settings_endpoint'] = ctx.settings_endpoint record['cron_tasks'] = {**(ctx.tasks or {}), **(ctx.cron_tasks or {})} record['onnx_providers'] = ctx.onnx_providers + record['analysis_providers'] = ctx.analysis_providers record['song_analyzed_hooks'] = ctx.song_analyzed_hooks if role == 'web' and flask_app is not None and ctx.blueprint is not None: if ctx.blueprint.name != plugin_id: @@ -854,6 +856,25 @@ def get_onnx_providers(self): providers.extend(record.get('onnx_providers', [])) return providers + def get_analysis_provider(self, component): + """Return the first loaded plugin's replacement for ``component``, or None. + + Resolves ``factory`` to the actual implementation (calling it when it is a + zero-arg callable). Used by core to let a plugin swap out a whole analysis + step such as the ASR/Whisper backend. + """ + for record in self.records.values(): + if record.get('load_status') not in _LOADED_STATUSES: + continue + factory = record.get('analysis_providers', {}).get(component) + if factory is None: + continue + try: + return factory() if callable(factory) else factory + except Exception: + logger.exception('Plugin analysis provider for %r failed to resolve', component) + return None + def song_analyzed_hooks(self): hooks = [] for record in self.records.values(): diff --git a/tasks/analysis/song.py b/tasks/analysis/song.py index 9053cb99a..5dd3e3445 100644 --- a/tasks/analysis/song.py +++ b/tasks/analysis/song.py @@ -103,7 +103,7 @@ def sigmoid(x): return 1 / (1 + np.exp(-x)) -def resolve_providers(allow_coreml=False, cuda_options=None): +def resolve_providers(allow_coreml=False, cuda_options=None, label=None): available = ort.get_available_providers() chain = [] @@ -134,8 +134,17 @@ def resolve_providers(allow_coreml=False, cuda_options=None): for provider in _plugin_onnx_providers(): name = provider.get('name') - if name and name in available and name not in [p[0] for p in chain]: - chain.append((name, provider.get('options') or {})) + if not name or name not in available or name in [p[0] for p in chain]: + continue + # Providers can be scoped to specific models by their session label, so a + # plugin can offer an accelerator for the graphs it handles + only = provider.get('only_models') + exclude = provider.get('exclude_models') + if only and label not in only: + continue + if exclude and label in exclude: + continue + chain.append((name, provider.get('options') or {})) chain.append(('CPUExecutionProvider', {})) logger.info("ONNX provider chain: %s", [p[0] for p in chain]) @@ -208,8 +217,10 @@ def _default_sess_options(): return opts -def create_onnx_session(model_path, provider_options=None, label="", sess_options=None): - opts = provider_options or resolve_providers() +def create_onnx_session( + model_path, provider_options=None, label="", sess_options=None, allow_coreml=False +): + opts = provider_options or resolve_providers(allow_coreml=allow_coreml, label=label) if sess_options is None: sess_options = _default_sess_options() try: @@ -229,7 +240,7 @@ def create_onnx_session(model_path, provider_options=None, label="", sess_option def load_musicnn_sessions(model_paths): - opts = resolve_providers(allow_coreml=False) + opts = resolve_providers(allow_coreml=False, label='musicnn') try: sessions = {n: create_onnx_session(p, opts, label=n) for n, p in model_paths.items()} logger.info(f"OK Loaded {len(sessions)} MusiCNN models for album reuse") diff --git a/tasks/clap_analyzer.py b/tasks/clap_analyzer.py index 3c3fa2b78..7639e2f36 100644 --- a/tasks/clap_analyzer.py +++ b/tasks/clap_analyzer.py @@ -49,8 +49,15 @@ _SEGMENT_LENGTH_SAMPLES = 480000 +# Providers whose graph compiler needs the CLAP audio model's symbolic +# time-frame axis pinned to a fixed value before it can compile the graph +# (neither compiles a dynamic dim). +_PREPARED_MODEL_PROVIDERS = {'CoreMLExecutionProvider', 'MIGraphXExecutionProvider'} -def _static_shape_model_bytes(model_path): + +def _prepared_model_bytes(model_path): + """Return the audio model with its symbolic time axis pinned to a static + shape, or None when the model has no symbolic dims. See _PREPARED_MODEL_PROVIDERS.""" import onnx from onnxruntime.tools.onnx_model_utils import make_dim_param_fixed @@ -115,6 +122,7 @@ def _load_audio_model(): 'arena_extend_strategy': 'kSameAsRequested', 'cudnn_conv_algo_search': 'DEFAULT', }, + label='clap', ) def _create_session(model_input, providers, provider_opts): @@ -131,14 +139,14 @@ def _create_session(model_input, providers, provider_opts): cpu_opts = [{}] preferred_model_input = model_path - if 'CoreMLExecutionProvider' in preferred_providers: + if _PREPARED_MODEL_PROVIDERS.intersection(preferred_providers): try: - static_bytes = _static_shape_model_bytes(model_path) - if static_bytes is not None: - preferred_model_input = static_bytes - logger.info("CoreML: pinned CLAP audio time axis to a static shape for compilation") + prepared_bytes = _prepared_model_bytes(model_path) + if prepared_bytes is not None: + preferred_model_input = prepared_bytes + logger.info("Pinned CLAP audio time axis to a static shape for graph compilation") except Exception as e: - logger.warning(f"CoreML: could not build static-shape model ({e}); using dynamic model") + logger.warning(f"Could not build static-shape model ({e}); using dynamic model") try: session = _create_session(preferred_model_input, preferred_providers, preferred_opts) diff --git a/test/unit/test_plugin_analysis_seams.py b/test/unit/test_plugin_analysis_seams.py new file mode 100644 index 000000000..b12f70781 --- /dev/null +++ b/test/unit/test_plugin_analysis_seams.py @@ -0,0 +1,237 @@ +# AudioMuse-AI - https://github.com/NeptuneHub/AudioMuse-AI +# Copyright (C) 2025 NeptuneHub +# SPDX-License-Identifier: AGPL-3.0-only +# +# This program is free software: you can redistribute it and/or modify it under +# the terms of the GNU Affero General Public License v3.0. See the LICENSE file +# in the project root or + +"""Unit tests for the two in-analysis plugin seams. + +Covers the extended ``register_onnx_provider`` (per-model scoping) and the newly +wired ``register_analysis_provider`` (component replacement), plus the code paths +that consume them: ``resolve_providers`` in the ONNX session builder and +``get_asr_backend`` in the lyrics pipeline. Everything here runs on CPU with fake +providers and stub backends, so the seams are testable without any GPU. + +Main Features: +* register_onnx_provider stores only_models/exclude_models and resolve_providers + honors them per session label, leaving unmatched models on the default chain +* PluginManager.get_onnx_providers only surfaces providers from loaded plugins +* register_analysis_provider/get_analysis_provider swap a whole component (asr), + resolving module objects and zero-arg factories and swallowing broken plugins +* lyrics.get_asr_backend prefers a registered override and falls back to built-in +""" + +import sys +import types + +import pytest + +import plugin.api as api +import plugin.manager as manager + + +def _record(plugin_id, load_status='ok', onnx_providers=None, analysis_providers=None): + return { + 'id': plugin_id, + 'name': plugin_id, + 'version': '1.0.0', + 'manifest': {}, + 'checksum': 'x', + 'requirements': [], + 'enabled': True, + 'settings': {}, + 'source_repo': None, + 'load_status': load_status, + 'menu_items': [], + 'cron_tasks': {}, + 'onnx_providers': onnx_providers or [], + 'analysis_providers': analysis_providers or {}, + 'error': None, + } + + +class TestRegisterOnnxProviderScoping: + def test_stores_scoping_fields(self): + ctx = api.PluginContext('demo', 'worker') + ctx.register_onnx_provider( + 'FakeGpuExecutionProvider', + {'device_id': 0}, + only_models=['musicnn', 'clap'], + ) + provider = ctx.onnx_providers[0] + assert provider['name'] == 'FakeGpuExecutionProvider' + assert provider['options'] == {'device_id': 0} + assert provider['only_models'] == ['musicnn', 'clap'] + assert provider['exclude_models'] is None + + def test_unscoped_provider_has_none_scopes(self): + ctx = api.PluginContext('demo', 'worker') + ctx.register_onnx_provider('FakeGpuExecutionProvider') + provider = ctx.onnx_providers[0] + assert provider['only_models'] is None + assert provider['exclude_models'] is None + assert provider['options'] == {} + + def test_scopes_are_copied_not_aliased(self): + ctx = api.PluginContext('demo', 'worker') + only = ['musicnn'] + ctx.register_onnx_provider('FakeGpuExecutionProvider', only_models=only) + only.append('clap') + assert ctx.onnx_providers[0]['only_models'] == ['musicnn'] + + +class TestResolveProvidersScoping: + """resolve_providers must apply plugin providers only to matching labels.""" + + @pytest.fixture + def song(self, monkeypatch): + song = pytest.importorskip('tasks.analysis.song') + fake_ort = types.SimpleNamespace( + get_available_providers=lambda: [ + 'FakeGpuExecutionProvider', 'CPUExecutionProvider' + ] + ) + monkeypatch.setattr(song, 'ort', fake_ort) + return song + + def _names(self, chain): + return [name for name, _opts in chain] + + def test_unscoped_provider_applies_to_every_label(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {'device_id': 0}, + 'position': 'before_cpu', 'only_models': None, 'exclude_models': None}, + ]) + for label in ('musicnn', 'clap', 'whisper_encoder', None): + names = self._names(song.resolve_providers(label=label)) + assert names == ['FakeGpuExecutionProvider', 'CPUExecutionProvider'] + + def test_only_models_limits_to_matching_label(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': ['musicnn'], 'exclude_models': None}, + ]) + assert 'FakeGpuExecutionProvider' in self._names(song.resolve_providers(label='musicnn')) + # clap is not in only_models, so it keeps the plain CPU chain. + assert self._names(song.resolve_providers(label='clap')) == ['CPUExecutionProvider'] + + def test_exclude_models_skips_matching_label(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': None, + 'exclude_models': ['whisper_encoder']}, + ]) + assert 'FakeGpuExecutionProvider' in self._names(song.resolve_providers(label='musicnn')) + assert self._names( + song.resolve_providers(label='whisper_encoder') + ) == ['CPUExecutionProvider'] + + def test_unavailable_provider_is_dropped(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'MissingExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': None, 'exclude_models': None}, + ]) + assert self._names(song.resolve_providers(label='musicnn')) == ['CPUExecutionProvider'] + + def test_cpu_provider_is_always_last(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': None, 'exclude_models': None}, + ]) + assert self._names(song.resolve_providers(label='musicnn'))[-1] == 'CPUExecutionProvider' + + +class TestManagerOnnxProviders: + def test_only_loaded_plugins_contribute(self): + mgr = manager.PluginManager() + loaded = {'name': 'FakeGpuExecutionProvider', 'options': {}, 'position': 'before_cpu', + 'only_models': ['musicnn'], 'exclude_models': None} + pending = {'name': 'OtherExecutionProvider', 'options': {}, 'position': 'before_cpu', + 'only_models': None, 'exclude_models': None} + mgr.records = { + 'ok_plugin': _record('ok_plugin', load_status='ok', onnx_providers=[loaded]), + 'unloaded': _record('unloaded', load_status=None, onnx_providers=[pending]), + } + providers = mgr.get_onnx_providers() + assert providers == [loaded] + + def test_deps_failed_status_still_contributes(self): + # 'deps_failed' is a loaded status: the plugin registered before pip failed. + mgr = manager.PluginManager() + provider = {'name': 'FakeGpuExecutionProvider', 'options': {}, 'position': 'before_cpu', + 'only_models': None, 'exclude_models': None} + mgr.records = {'p': _record('p', load_status='deps_failed', onnx_providers=[provider])} + assert mgr.get_onnx_providers() == [provider] + + +class TestRegisterAnalysisProvider: + def test_context_stores_factory(self): + ctx = api.PluginContext('demo', 'worker') + backend = object() + ctx.register_analysis_provider('asr', backend) + assert ctx.analysis_providers == {'asr': backend} + + def test_manager_returns_module_object_directly(self): + mgr = manager.PluginManager() + backend = object() + mgr.records = {'p': _record('p', analysis_providers={'asr': backend})} + assert mgr.get_analysis_provider('asr') is backend + + def test_manager_resolves_zero_arg_factory(self): + mgr = manager.PluginManager() + backend = object() + mgr.records = {'p': _record('p', analysis_providers={'asr': lambda: backend})} + assert mgr.get_analysis_provider('asr') is backend + + def test_unregistered_component_returns_none(self): + mgr = manager.PluginManager() + mgr.records = {'p': _record('p', analysis_providers={'asr': object()})} + assert mgr.get_analysis_provider('embedding') is None + + def test_unloaded_plugin_is_not_consulted(self): + mgr = manager.PluginManager() + backend = object() + mgr.records = {'p': _record('p', load_status=None, analysis_providers={'asr': backend})} + assert mgr.get_analysis_provider('asr') is None + + def test_broken_factory_is_swallowed(self): + mgr = manager.PluginManager() + + def boom(): + raise RuntimeError('backend import failed') + + mgr.records = {'p': _record('p', analysis_providers={'asr': boom})} + assert mgr.get_analysis_provider('asr') is None + + +class TestGetAsrBackend: + @pytest.fixture + def asr(self): + return pytest.importorskip('lyrics._asr_backend') + + def test_override_is_used_when_registered(self, asr, monkeypatch): + backend = object() + monkeypatch.setattr( + manager.plugin_manager, 'get_analysis_provider', + lambda component: backend if component == 'asr' else None, + ) + assert asr.get_asr_backend() is backend + + def test_falls_back_to_builtin_when_no_override(self, asr, monkeypatch): + monkeypatch.setattr( + manager.plugin_manager, 'get_analysis_provider', lambda component: None + ) + stub = types.ModuleType('lyrics.whisper_onnx') + monkeypatch.setitem(sys.modules, 'lyrics.whisper_onnx', stub) + assert asr.get_asr_backend() is stub + + def test_manager_error_falls_back_to_builtin(self, asr, monkeypatch): + def boom(component): + raise RuntimeError('plugin manager exploded') + + monkeypatch.setattr(manager.plugin_manager, 'get_analysis_provider', boom) + stub = types.ModuleType('lyrics.whisper_onnx') + monkeypatch.setitem(sys.modules, 'lyrics.whisper_onnx', stub) + assert asr.get_asr_backend() is stub From 0be88f6a8ef93a2fc415d4485c852b4c0f2aa51d Mon Sep 17 00:00:00 2001 From: Schaka <2223171+Schaka@users.noreply.github.com> Date: Sun, 26 Jul 2026 09:22:54 +0200 Subject: [PATCH 2/5] [Plugin] Rework system to allow plugins to add execution providers more dynamically --- .gitignore | 3 + docs/PLUGIN.md | 33 ++- lyrics/_asr_backend.py | 28 ++- lyrics/gte_onnx.py | 4 +- lyrics/silero_onnx.py | 4 +- plugin/api.py | 76 ++++++- plugin/manager.py | 43 +++- tasks/analysis/song.py | 68 +++++- tasks/clap_analyzer.py | 48 +++-- test/unit/test_plugin_analysis_seams.py | 274 +++++++++++++++++++++++- 10 files changed, 514 insertions(+), 67 deletions(-) diff --git a/.gitignore b/.gitignore index 612f23436..ff93b452d 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,8 @@ env/ # IDE and editor configuration .vscode/ .idea/ +.tokensave +.claude/.headroom_wrap_marker.json # macOS Finder / metadata files .DS_Store @@ -76,3 +78,4 @@ analysis.md .claude/ lyrics_model_gte_vnni.tar.gz native-build/windows/vendor/redis/amd64/redis-server.exe +.venv-test \ No newline at end of file diff --git a/docs/PLUGIN.md b/docs/PLUGIN.md index 1df0c2060..1b0683942 100644 --- a/docs/PLUGIN.md +++ b/docs/PLUGIN.md @@ -469,8 +469,37 @@ And the methods on the `ctx` object in `register(ctx)`: | `add_task(name, func, queue='default')` | Register a named worker task. It appears on the Scheduled Tasks page like a cron task, so the admin can schedule it or run it now. | | `on_install(func)` | Run once at install and on every update. Gets the database connection. | | `on_flask_start(func)` / `on_worker_start(func)` | Run at every start of that container, after the plugin loads. | -| `register_onnx_provider(name, options, position, only_models=None, exclude_models=None)` | Advanced: offer an extra ONNX Runtime execution provider (for example a GPU) for analysis. Scope it to specific models with `only_models`/`exclude_models` (lists of the session `label`: `musicnn`, `clap`, `whisper_encoder`, ...) when the provider can only parse some graphs. | -| `register_analysis_provider(component, factory)` | Advanced: replace a whole analysis component with a plugin-supplied implementation. `component` is currently `asr` (the Whisper backend); `factory` is the replacement module/object (or a zero-arg callable returning one) matching the built-in surface `load_whisper_model`/`transcribe`/`is_loaded`/`unload`. Consulted before the built-in. | +| `register_onnx_provider(name, options, position, only_models=None, exclude_models=None, needs_static_shapes=False)` | Advanced: offer an extra ONNX Runtime execution provider (for example a GPU) for analysis. See "Offer an ONNX execution provider" below. | +| `register_analysis_provider(component, factory, cache=True)` | Advanced: replace a whole analysis component with a plugin-supplied implementation. `component` is currently `asr` (the Whisper backend); `factory` is the replacement module/object (or a zero-arg callable returning one) matching the built-in surface `load_whisper_model`/`transcribe`/`is_loaded`/`unload`. Consulted before the built-in; an unknown `component` or a backend missing part of that surface is ignored with a warning in the logs, and the built-in is used. `cache=True` (the default) resolves a callable `factory` once and reuses the result, like the built-in modules, which stay loaded for a whole album; pass `cache=False` to be called for every use, and then unload what you hand out yourself. | + +### Offer an ONNX execution provider + +`register_onnx_provider` adds your provider to the chain AudioMuse-AI builds for every analysis model. `position` is `before_cpu` (the default) or `before_cuda` when your provider should win over CUDA. Set `needs_static_shapes=True` if your graph compiler cannot handle symbolic dimensions, so core pins them before it builds the session (the CLAP audio model needs this). + +Scope the provider to the models it can actually compile with `only_models`/`exclude_models`. Both take a session label or a list of them; an unknown label is logged as a warning. The labels are: + +| Label | Model | Default without a plugin | +|---|---|---| +| `musicnn` | MusiCNN embedding + prediction | CUDA, else CPU | +| `clap` | CLAP audio model | CUDA / CoreML, else CPU | +| `clap_text` | CLAP text model, on the worker | CUDA, else CPU | +| `whisper_encoder` | Whisper encoder (lyrics) | CUDA, else CPU | +| `whisper_decoder` | Whisper decoder (lyrics) | CUDA, else CPU | +| `gte` | GTE lyrics text embedding | CPU only | +| `silero_vad` | Silero voice activity detection | CPU only | + +The last two are CPU by default and stay that way unless a plugin names them in `only_models`, so nothing changes for a normal user. The CLAP text model runs on CPU in the Flask process for thread safety and is not offered to plugins there; `clap_text` applies on the worker only. + +```python +def register(ctx): + # A provider that handles MusiCNN and the Whisper encoder, but not the + # decoder and not CLAP. + ctx.register_onnx_provider( + 'MyExecutionProvider', + {'device_id': 0}, + only_models=['musicnn', 'whisper_encoder'], + ) +``` ## Who can see and manage plugins diff --git a/lyrics/_asr_backend.py b/lyrics/_asr_backend.py index 2e28dc410..89e120330 100644 --- a/lyrics/_asr_backend.py +++ b/lyrics/_asr_backend.py @@ -17,16 +17,23 @@ Main Features: * Consults the loaded plugins for an 'asr' analysis provider before the built-in -* Falls back to the built-in whisper_onnx backend when no plugin registered one +* Falls back to the built-in whisper_onnx backend when no plugin registered one, + or when the replacement is missing part of the required surface * Swallows any plugin-resolution error so a broken plugin never breaks lyrics """ from __future__ import annotations +import logging + +logger = logging.getLogger(__name__) + +REQUIRED_METHODS = ('load_whisper_model', 'transcribe', 'is_loaded', 'unload') + def get_asr_backend(): override = _plugin_asr_backend() - if override is not None: + if override is not None and _has_required_surface(override): return override from . import whisper_onnx @@ -40,3 +47,20 @@ def _plugin_asr_backend(): return plugin_manager.get_analysis_provider('asr') except Exception: return None + + +# Rejects a backend that cannot stand in for whisper_onnx. A missing method would +# surface far from here: a broken is_loaded makes is_lyrics_loaded() report True +# forever, so every album pays a full memory cleanup for nothing, and a broken +# transcribe fails every song with no way back to the built-in. +def _has_required_surface(backend): + missing = [ + name for name in REQUIRED_METHODS if not callable(getattr(backend, name, None)) + ] + if not missing: + return True + logger.warning( + 'Plugin ASR backend %r is missing %s; using the built-in whisper instead', + backend, ', '.join(missing), + ) + return False diff --git a/lyrics/gte_onnx.py b/lyrics/gte_onnx.py index 17ee694cd..e61336acf 100644 --- a/lyrics/gte_onnx.py +++ b/lyrics/gte_onnx.py @@ -98,11 +98,11 @@ def load_gte_model(): sess_options.intra_op_num_threads = max(1, (os.cpu_count() or 2) // 2) sess_options.inter_op_num_threads = 1 try: - from tasks.analysis.song import create_onnx_session + from tasks.analysis.song import create_onnx_session, resolve_providers session = create_onnx_session( onnx_path, - provider_options=[('CPUExecutionProvider', {})], + provider_options=resolve_providers(label='gte', cpu_only_default=True), sess_options=sess_options, label='gte', ) diff --git a/lyrics/silero_onnx.py b/lyrics/silero_onnx.py index 846890ac3..68436ec4e 100644 --- a/lyrics/silero_onnx.py +++ b/lyrics/silero_onnx.py @@ -68,11 +68,11 @@ def _load_session(model_path: Optional[str] = None): opts.enable_cpu_mem_arena = False opts.enable_mem_pattern = False try: - from tasks.analysis.song import create_onnx_session + from tasks.analysis.song import create_onnx_session, resolve_providers _session = create_onnx_session( path, - provider_options=[('CPUExecutionProvider', {})], + provider_options=resolve_providers(label='silero_vad', cpu_only_default=True), sess_options=opts, label='silero_vad', ) diff --git a/plugin/api.py b/plugin/api.py index b887e7e4c..183bb3941 100644 --- a/plugin/api.py +++ b/plugin/api.py @@ -50,6 +50,12 @@ _ID_RE = re.compile(r'^[a-z][a-z0-9_]{1,63}$') _NAME_RE = re.compile(r'^[a-z][a-z0-9_]{0,62}$') +# Analysis steps a plugin may replace wholesale with register_analysis_provider. +ANALYSIS_COMPONENTS = frozenset({'asr'}) + +# Where a plugin provider goes in the ONNX chain, see register_onnx_provider. +ONNX_POSITIONS = frozenset({'before_cuda', 'before_cpu'}) + __all__ = [ 'PluginContext', 'config', 'logger', 'get_db', 'save_task_status', 'get_score_data_by_ids', 'get_tracks_by_ids', 'get_setting', 'set_setting', @@ -181,6 +187,19 @@ def use_server(server_id): return ms_context.use_server(ms_registry.context_for(server_id) if server_id else None) +def _model_scope(value): + """Normalize an only_models/exclude_models argument to a list or None. + + A single label is accepted as a plain string; without this ``'musicnn'`` + would become ``['m', 'u', 's', ...]`` and silently match nothing. + """ + if not value: + return None + if isinstance(value, str): + return [value] + return list(value) + + class PluginContext: """Registration sink passed to a plugin's ``register(ctx)``. @@ -227,13 +246,30 @@ def add_cron_task(self, name, func, queue='default'): self.cron_tasks[name] = {'dotted': dotted_path(func), 'queue': queue} def register_onnx_provider(self, name, options=None, position='before_cpu', - only_models=None, exclude_models=None): + only_models=None, exclude_models=None, + needs_static_shapes=False): + """Offer an extra ONNX Runtime execution provider for the analysis sessions. + + ``only_models``/``exclude_models`` scope the provider to specific session + labels (a single label may be given as a plain string). ``position`` is + ``'before_cpu'`` (the default) or ``'before_cuda'``. Set + ``needs_static_shapes`` when the provider's graph compiler cannot handle + symbolic dimensions, so core pins them before building the session. + """ + if position not in ONNX_POSITIONS: + logger.warning( + "Plugin %s registered ONNX provider %s with unknown position %r; " + "using 'before_cpu'. Valid positions: %s", + self.plugin_id, name, position, sorted(ONNX_POSITIONS), + ) + position = 'before_cpu' self.onnx_providers.append({ 'name': name, 'options': options or {}, 'position': position, - 'only_models': list(only_models) if only_models else None, - 'exclude_models': list(exclude_models) if exclude_models else None, + 'only_models': _model_scope(only_models), + 'exclude_models': _model_scope(exclude_models), + 'needs_static_shapes': bool(needs_static_shapes), }) def on_flask_start(self, func): @@ -257,18 +293,36 @@ def on_song_analyzed(self, func): def on_install(self, func): self.install_hooks.append(func) - def register_analysis_provider(self, component, factory): + def register_analysis_provider(self, component, factory, cache=True): """Replace a whole analysis component with a plugin-supplied implementation. Some accelerators need more than a different ONNX execution provider: they need a different library entirely. MIGraphX, for instance, cannot run the ONNX Whisper decoder at all, so an AMD plugin swaps in faster-whisper. - ``component`` names the step to replace (currently ``asr``). ``factory`` is - the replacement module/object, or a zero-arg callable returning one. It must - match the built-in module's public surface; for ``asr`` that is - ``load_whisper_model()``, ``transcribe(wav, sr, language=None)``, - ``is_loaded()`` and ``unload()``. Core consults the registered provider - first and falls back to the built-in when no plugin registered one. + ``component`` names the step to replace, one of ANALYSIS_COMPONENTS. + ``factory`` is the replacement module/object, or a zero-arg callable + returning one. It must match the built-in module's public surface; for + ``asr`` that is ``load_whisper_model()``, ``transcribe(wav, sr, + language=None)``, ``is_loaded()`` and ``unload()``. Core consults the + registered provider first and falls back to the built-in when no plugin + registered one, or when the replacement is missing part of that surface. + + A callable ``factory`` is resolved once and the result reused, matching the + built-in modules, which stay loaded for a whole album and are freed at its + end. Pass ``cache=False`` to be called for every use instead; then the + plugin owns unloading whatever it hands out. """ - self.analysis_providers[component] = factory + if component not in ANALYSIS_COMPONENTS: + logger.warning( + 'Plugin %s registered an analysis provider for unknown component %r; ' + 'ignoring it. Known components: %s', + self.plugin_id, component, sorted(ANALYSIS_COMPONENTS), + ) + return + if component in self.analysis_providers: + logger.warning( + 'Plugin %s registered two analysis providers for %r; keeping the last one', + self.plugin_id, component, + ) + self.analysis_providers[component] = {'factory': factory, 'cache': bool(cache)} diff --git a/plugin/manager.py b/plugin/manager.py index 60ea188f8..a11f42961 100644 --- a/plugin/manager.py +++ b/plugin/manager.py @@ -190,6 +190,13 @@ def _valid_requirement(spec): _LOADED_STATUSES = ('ok', 'deps_failed') +def _analysis_provider_entry(value): + """Normalize a stored analysis provider to ``{'factory': ..., 'cache': bool}``.""" + if isinstance(value, dict) and 'factory' in value: + return {'factory': value['factory'], 'cache': bool(value.get('cache', True))} + return {'factory': value, 'cache': True} + + def _plugin_targets(manifest): raw = (manifest or {}).get('targets') if not raw: @@ -272,6 +279,7 @@ def __init__(self): self._boot_snapshot = None self._runtime_dirty = False self.last_pip_error = None + self._analysis_provider_cache = {} def enabled(self): return bool(config.PLUGINS_ENABLED) @@ -309,6 +317,7 @@ def _runs_here(self, record, role): return _role_target(role) in _plugin_targets(record.get('manifest')) def sync(self, conn=None, role=None): + self._analysis_provider_cache = {} if not self.enabled(): self.records = {} return @@ -860,19 +869,35 @@ def get_analysis_provider(self, component): """Return the first loaded plugin's replacement for ``component``, or None. Resolves ``factory`` to the actual implementation (calling it when it is a - zero-arg callable). Used by core to let a plugin swap out a whole analysis - step such as the ASR/Whisper backend. + zero-arg callable) and reuses that result unless the plugin registered with + ``cache=False``, so a model is not rebuilt on every call. Used by core to + let a plugin swap out a whole analysis step such as the ASR/Whisper backend. """ - for record in self.records.values(): - if record.get('load_status') not in _LOADED_STATUSES: - continue - factory = record.get('analysis_providers', {}).get(component) - if factory is None: - continue + if component in self._analysis_provider_cache: + return self._analysis_provider_cache[component] + owners = [ + plugin_id for plugin_id, record in self.records.items() + if record.get('load_status') in _LOADED_STATUSES + and record.get('analysis_providers', {}).get(component) is not None + ] + if len(owners) > 1: + logger.warning( + 'Plugins %s all provide the analysis component %r; using %s', + ', '.join(owners), component, owners[0], + ) + for plugin_id in owners: + entry = _analysis_provider_entry( + self.records[plugin_id]['analysis_providers'][component] + ) try: - return factory() if callable(factory) else factory + factory = entry['factory'] + provider = factory() if callable(factory) else factory except Exception: logger.exception('Plugin analysis provider for %r failed to resolve', component) + continue + if entry['cache']: + self._analysis_provider_cache[component] = provider + return provider return None def song_analyzed_hooks(self): diff --git a/tasks/analysis/song.py b/tasks/analysis/song.py index 4b6b22c65..58c8c9978 100644 --- a/tasks/analysis/song.py +++ b/tasks/analysis/song.py @@ -104,11 +104,60 @@ def sigmoid(x): return 1 / (1 + np.exp(-x)) -def resolve_providers(allow_coreml=False, cuda_options=None, label=None): +# The session labels a plugin can scope an ONNX provider to. Every session that +# calls resolve_providers passes one of these, so anything else in a plugin's +# only_models/exclude_models is a typo that would silently match nothing. +MODEL_LABELS = frozenset({ + 'musicnn', + 'clap', + 'clap_text', + 'whisper_encoder', + 'whisper_decoder', + 'gte', + 'silero_vad', +}) + +def _scoped_labels(provider, key): + """Return a provider's model scope as a list, warning about unknown labels.""" + value = provider.get(key) + if not value: + return None + labels = [value] if isinstance(value, str) else list(value) + unknown = [name for name in labels if name not in MODEL_LABELS] + if unknown: + logger.warning( + "ONNX provider %s: unknown %s %s - known model labels are %s", + provider.get('name'), key, unknown, sorted(MODEL_LABELS), + ) + return labels + + +def _add_plugin_provider(chain, provider, entry): + position = provider.get('position') or 'before_cpu' + if position == 'before_cuda': + chain.insert(0, entry) + return + if position != 'before_cpu': + logger.warning( + "ONNX provider %s: unknown position %r - using 'before_cpu'", + provider.get('name'), position, + ) + chain.append(entry) + + +def resolve_providers(allow_coreml=False, cuda_options=None, label=None, + cpu_only_default=False): + """Build the ONNX provider chain for one session. + + ``label`` names the model (see MODEL_LABELS) so a plugin can offer its + accelerator for only the graphs it can compile. ``cpu_only_default`` marks a + session that core keeps on CPU: the built-in GPU providers are skipped and a + plugin provider is used only when it names the label in ``only_models``. + """ available = ort.get_available_providers() chain = [] - if 'CUDAExecutionProvider' in available: + if not cpu_only_default and 'CUDAExecutionProvider' in available: chain.append( ( 'CUDAExecutionProvider', @@ -122,7 +171,7 @@ def resolve_providers(allow_coreml=False, cuda_options=None, label=None): ) ) - if allow_coreml and 'CoreMLExecutionProvider' in available: + if not cpu_only_default and allow_coreml and 'CoreMLExecutionProvider' in available: chain.append( ( 'CoreMLExecutionProvider', @@ -139,16 +188,19 @@ def resolve_providers(allow_coreml=False, cuda_options=None, label=None): continue # Providers can be scoped to specific models by their session label, so a # plugin can offer an accelerator for the graphs it handles - only = provider.get('only_models') - exclude = provider.get('exclude_models') + only = _scoped_labels(provider, 'only_models') + exclude = _scoped_labels(provider, 'exclude_models') if only and label not in only: continue if exclude and label in exclude: continue - chain.append((name, provider.get('options') or {})) + # A CPU-by-default session is opt-in: the plugin has to name it. + if cpu_only_default and not only: + continue + _add_plugin_provider(chain, provider, (name, provider.get('options') or {})) chain.append(('CPUExecutionProvider', {})) - logger.info("ONNX provider chain: %s", [p[0] for p in chain]) + logger.info("ONNX provider chain for %s: %s", label or 'unlabelled', [p[0] for p in chain]) return chain @@ -425,7 +477,7 @@ def _patches_for_track(audio, sr, name): def _sessions_for_track(onnx_sessions, model_paths): if onnx_sessions: return onnx_sessions['embedding'], onnx_sessions['prediction'], False - provider_options = resolve_providers() + provider_options = resolve_providers(label='musicnn') embedding_sess = create_onnx_session( model_paths['embedding'], provider_options, label='embedding' ) diff --git a/tasks/clap_analyzer.py b/tasks/clap_analyzer.py index 7639e2f36..ede23b2ec 100644 --- a/tasks/clap_analyzer.py +++ b/tasks/clap_analyzer.py @@ -51,8 +51,22 @@ # Providers whose graph compiler needs the CLAP audio model's symbolic # time-frame axis pinned to a fixed value before it can compile the graph -# (neither compiles a dynamic dim). -_PREPARED_MODEL_PROVIDERS = {'CoreMLExecutionProvider', 'MIGraphXExecutionProvider'} +# (none of them compiles a dynamic dim). Plugin providers join this set by +# registering with needs_static_shapes=True, so core never has to know their names. +_PREPARED_MODEL_PROVIDERS = {'CoreMLExecutionProvider'} + + +def _static_shape_providers(): + providers = set(_PREPARED_MODEL_PROVIDERS) + try: + from plugin.manager import plugin_manager + + for provider in plugin_manager.get_onnx_providers(): + if provider.get('needs_static_shapes') and provider.get('name'): + providers.add(provider['name']) + except Exception: + logger.debug('Could not read plugin ONNX providers', exc_info=True) + return providers def _prepared_model_bytes(model_path): @@ -139,7 +153,7 @@ def _create_session(model_input, providers, provider_opts): cpu_opts = [{}] preferred_model_input = model_path - if _PREPARED_MODEL_PROVIDERS.intersection(preferred_providers): + if _static_shape_providers().intersection(preferred_providers): try: prepared_bytes = _prepared_model_bytes(model_path) if prepared_bytes is not None: @@ -198,7 +212,6 @@ def _load_text_model(): sess_options = _clap_session_options("Text") session = None - available_providers = ort.get_available_providers() _is_worker = os.environ.get('AUDIOMUSE_ROLE') == 'worker' if not _is_worker: @@ -206,24 +219,17 @@ def _load_text_model(): logger.info( "CLAP text model: CPU only (Flask process) - thread-safe across request threads" ) - elif 'CUDAExecutionProvider' in available_providers: - gpu_device_id = 0 - cuda_visible = os.environ.get('CUDA_VISIBLE_DEVICES', '') - if cuda_visible and cuda_visible != '-1': - gpu_device_id = 0 - - cuda_options = { - 'device_id': gpu_device_id, - 'arena_extend_strategy': 'kSameAsRequested', - 'cudnn_conv_algo_search': 'DEFAULT', - } - provider_options = [('CUDAExecutionProvider', cuda_options), ('CPUExecutionProvider', {})] - logger.info( - f"CUDA provider available - will attempt to use GPU (device_id={gpu_device_id})" - ) else: - provider_options = [('CPUExecutionProvider', {})] - logger.info("CUDA provider not available - using CPU only") + from tasks.analysis.song import resolve_providers + + provider_options = resolve_providers( + cuda_options={ + 'device_id': 0, + 'arena_extend_strategy': 'kSameAsRequested', + 'cudnn_conv_algo_search': 'DEFAULT', + }, + label='clap_text', + ) try: session = ort.InferenceSession( diff --git a/test/unit/test_plugin_analysis_seams.py b/test/unit/test_plugin_analysis_seams.py index b12f70781..cd7e8c89e 100644 --- a/test/unit/test_plugin_analysis_seams.py +++ b/test/unit/test_plugin_analysis_seams.py @@ -15,12 +15,17 @@ providers and stub backends, so the seams are testable without any GPU. Main Features: -* register_onnx_provider stores only_models/exclude_models and resolve_providers - honors them per session label, leaving unmatched models on the default chain +* register_onnx_provider stores only_models/exclude_models (a single label may be + a plain string), position and needs_static_shapes, and resolve_providers honors + them per session label, leaving unmatched models on the default chain +* Sessions core keeps on CPU (gte, silero_vad) stay on CPU unless a plugin opts in + by naming them in only_models * PluginManager.get_onnx_providers only surfaces providers from loaded plugins * register_analysis_provider/get_analysis_provider swap a whole component (asr), - resolving module objects and zero-arg factories and swallowing broken plugins -* lyrics.get_asr_backend prefers a registered override and falls back to built-in + resolving module objects and zero-arg factories once (unless cache=False), + rejecting unknown components and swallowing broken plugins +* lyrics.get_asr_backend prefers a registered override, and falls back to the + built-in when there is none or the override misses part of the whisper surface """ import sys @@ -81,6 +86,31 @@ def test_scopes_are_copied_not_aliased(self): only.append('clap') assert ctx.onnx_providers[0]['only_models'] == ['musicnn'] + def test_single_label_may_be_a_plain_string(self): + ctx = api.PluginContext('demo', 'worker') + ctx.register_onnx_provider( + 'FakeGpuExecutionProvider', only_models='musicnn', exclude_models='clap' + ) + provider = ctx.onnx_providers[0] + assert provider['only_models'] == ['musicnn'] + assert provider['exclude_models'] == ['clap'] + + def test_unknown_position_falls_back_to_before_cpu(self, caplog): + ctx = api.PluginContext('demo', 'worker') + ctx.register_onnx_provider('FakeGpuExecutionProvider', position='after_everything') + assert ctx.onnx_providers[0]['position'] == 'before_cpu' + assert 'unknown position' in caplog.text + + def test_static_shapes_flag_is_stored(self): + ctx = api.PluginContext('demo', 'worker') + ctx.register_onnx_provider('FakeGpuExecutionProvider', needs_static_shapes=True) + assert ctx.onnx_providers[0]['needs_static_shapes'] is True + + def test_static_shapes_defaults_to_false(self): + ctx = api.PluginContext('demo', 'worker') + ctx.register_onnx_provider('FakeGpuExecutionProvider') + assert ctx.onnx_providers[0]['needs_static_shapes'] is False + class TestResolveProvidersScoping: """resolve_providers must apply plugin providers only to matching labels.""" @@ -96,6 +126,15 @@ def song(self, monkeypatch): monkeypatch.setattr(song, 'ort', fake_ort) return song + @pytest.fixture + def song_with_cuda(self, song, monkeypatch): + monkeypatch.setattr(song, 'ort', types.SimpleNamespace( + get_available_providers=lambda: [ + 'CUDAExecutionProvider', 'FakeGpuExecutionProvider', 'CPUExecutionProvider' + ] + )) + return song + def _names(self, chain): return [name for name, _opts in chain] @@ -142,6 +181,126 @@ def test_cpu_provider_is_always_last(self, song, monkeypatch): ]) assert self._names(song.resolve_providers(label='musicnn'))[-1] == 'CPUExecutionProvider' + def test_string_scope_is_accepted(self, song, monkeypatch): + # A provider stored by an older plugin (or by hand) may carry a bare string. + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': 'musicnn', 'exclude_models': None}, + ]) + assert 'FakeGpuExecutionProvider' in self._names(song.resolve_providers(label='musicnn')) + assert self._names(song.resolve_providers(label='clap')) == ['CPUExecutionProvider'] + + def test_unknown_label_is_reported(self, song, monkeypatch, caplog): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': ['musicnnn'], 'exclude_models': None}, + ]) + song.resolve_providers(label='musicnn') + assert 'unknown only_models' in caplog.text + + def test_known_labels_are_not_reported(self, song, monkeypatch, caplog): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, 'position': 'before_cpu', + 'only_models': sorted(song.MODEL_LABELS), 'exclude_models': None}, + ]) + song.resolve_providers(label='musicnn') + assert 'unknown' not in caplog.text + + def test_before_cuda_position_wins_over_cuda(self, song_with_cuda, monkeypatch): + monkeypatch.setattr(song_with_cuda, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cuda', 'only_models': None, 'exclude_models': None}, + ]) + assert self._names(song_with_cuda.resolve_providers(label='musicnn')) == [ + 'FakeGpuExecutionProvider', 'CUDAExecutionProvider', 'CPUExecutionProvider' + ] + + def test_before_cpu_position_follows_cuda(self, song_with_cuda, monkeypatch): + monkeypatch.setattr(song_with_cuda, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': None, 'exclude_models': None}, + ]) + assert self._names(song_with_cuda.resolve_providers(label='musicnn')) == [ + 'CUDAExecutionProvider', 'FakeGpuExecutionProvider', 'CPUExecutionProvider' + ] + + +class TestCpuOnlyDefaultSessions: + """Sessions core keeps on CPU stay there unless a plugin asks for them.""" + + @pytest.fixture + def song(self, monkeypatch): + song = pytest.importorskip('tasks.analysis.song') + monkeypatch.setattr(song, 'ort', types.SimpleNamespace( + get_available_providers=lambda: [ + 'CUDAExecutionProvider', 'FakeGpuExecutionProvider', 'CPUExecutionProvider' + ] + )) + return song + + def _names(self, chain): + return [name for name, _opts in chain] + + def test_builtin_gpu_providers_are_skipped(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: []) + chain = song.resolve_providers(label='gte', cpu_only_default=True) + assert self._names(chain) == ['CPUExecutionProvider'] + + def test_unscoped_plugin_provider_is_not_used(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': None, 'exclude_models': None}, + ]) + chain = song.resolve_providers(label='gte', cpu_only_default=True) + assert self._names(chain) == ['CPUExecutionProvider'] + + def test_plugin_provider_can_opt_in_by_label(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': ['gte'], 'exclude_models': None}, + ]) + chain = song.resolve_providers(label='gte', cpu_only_default=True) + assert self._names(chain) == ['FakeGpuExecutionProvider', 'CPUExecutionProvider'] + + def test_opt_in_for_another_label_does_not_leak(self, song, monkeypatch): + monkeypatch.setattr(song, '_plugin_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'options': {}, + 'position': 'before_cpu', 'only_models': ['gte'], 'exclude_models': None}, + ]) + chain = song.resolve_providers(label='silero_vad', cpu_only_default=True) + assert self._names(chain) == ['CPUExecutionProvider'] + + +class TestStaticShapeProviders: + """CLAP must learn which providers need pinned shapes from the registration.""" + + @pytest.fixture + def clap(self): + return pytest.importorskip('tasks.clap_analyzer') + + def test_plugin_provider_opts_in(self, clap, monkeypatch): + monkeypatch.setattr(manager.plugin_manager, 'get_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'needs_static_shapes': True}, + ]) + assert 'FakeGpuExecutionProvider' in clap._static_shape_providers() + + def test_provider_without_the_flag_is_left_out(self, clap, monkeypatch): + monkeypatch.setattr(manager.plugin_manager, 'get_onnx_providers', lambda: [ + {'name': 'FakeGpuExecutionProvider', 'needs_static_shapes': False}, + ]) + assert 'FakeGpuExecutionProvider' not in clap._static_shape_providers() + + def test_builtin_coreml_is_always_there(self, clap, monkeypatch): + monkeypatch.setattr(manager.plugin_manager, 'get_onnx_providers', lambda: []) + assert 'CoreMLExecutionProvider' in clap._static_shape_providers() + + def test_broken_plugin_manager_is_survivable(self, clap, monkeypatch): + def boom(): + raise RuntimeError('plugin manager exploded') + + monkeypatch.setattr(manager.plugin_manager, 'get_onnx_providers', boom) + assert clap._static_shape_providers() == {'CoreMLExecutionProvider'} + class TestManagerOnnxProviders: def test_only_loaded_plugins_contribute(self): @@ -171,7 +330,64 @@ def test_context_stores_factory(self): ctx = api.PluginContext('demo', 'worker') backend = object() ctx.register_analysis_provider('asr', backend) - assert ctx.analysis_providers == {'asr': backend} + assert ctx.analysis_providers == {'asr': {'factory': backend, 'cache': True}} + + def test_cache_opt_out_is_stored(self): + ctx = api.PluginContext('demo', 'worker') + ctx.register_analysis_provider('asr', object(), cache=False) + assert ctx.analysis_providers['asr']['cache'] is False + + def test_unknown_component_is_ignored(self, caplog): + ctx = api.PluginContext('demo', 'worker') + ctx.register_analysis_provider('ASR', object()) + assert ctx.analysis_providers == {} + assert 'unknown component' in caplog.text + + def test_double_registration_warns_and_keeps_last(self, caplog): + ctx = api.PluginContext('demo', 'worker') + first, second = object(), object() + ctx.register_analysis_provider('asr', first) + ctx.register_analysis_provider('asr', second) + assert ctx.analysis_providers['asr']['factory'] is second + assert 'two analysis providers' in caplog.text + + def test_two_plugins_for_one_component_warn(self, caplog): + mgr = manager.PluginManager() + first, second = object(), object() + mgr.records = { + 'a': _record('a', analysis_providers={'asr': first}), + 'b': _record('b', analysis_providers={'asr': second}), + } + assert mgr.get_analysis_provider('asr') is first + assert 'all provide the analysis component' in caplog.text + + def test_cached_factory_runs_once(self): + mgr = manager.PluginManager() + calls = [] + + def factory(): + calls.append(1) + return object() + + mgr.records = {'p': _record('p', analysis_providers={ + 'asr': {'factory': factory, 'cache': True}, + })} + assert mgr.get_analysis_provider('asr') is mgr.get_analysis_provider('asr') + assert len(calls) == 1 + + def test_uncached_factory_runs_every_time(self): + mgr = manager.PluginManager() + mgr.records = {'p': _record('p', analysis_providers={ + 'asr': {'factory': object, 'cache': False}, + })} + assert mgr.get_analysis_provider('asr') is not mgr.get_analysis_provider('asr') + + def test_sync_clears_the_cache(self, monkeypatch): + mgr = manager.PluginManager() + mgr._analysis_provider_cache = {'asr': object()} + monkeypatch.setattr(mgr, 'enabled', lambda: False) + mgr.sync() + assert mgr._analysis_provider_cache == {} def test_manager_returns_module_object_directly(self): mgr = manager.PluginManager() @@ -211,20 +427,59 @@ class TestGetAsrBackend: def asr(self): return pytest.importorskip('lyrics._asr_backend') + def _backend(self, **overrides): + """A stand-in with the full whisper surface, minus anything overridden away.""" + surface = {name: (lambda *a, **k: None) for name in ('load_whisper_model', + 'transcribe', 'is_loaded', 'unload')} + surface.update(overrides) + return types.SimpleNamespace(**{k: v for k, v in surface.items() if v is not None}) + + def _stub_builtin(self, monkeypatch): + """Replace the built-in whisper backend with an empty module. + + ``from . import whisper_onnx`` reads the attribute off the package once + the submodule has been imported by anything else in the session, so + patching sys.modules alone is not enough. + """ + stub = types.ModuleType('lyrics.whisper_onnx') + lyrics_pkg = pytest.importorskip('lyrics') + monkeypatch.setitem(sys.modules, 'lyrics.whisper_onnx', stub) + monkeypatch.setattr(lyrics_pkg, 'whisper_onnx', stub, raising=False) + return stub + def test_override_is_used_when_registered(self, asr, monkeypatch): - backend = object() + backend = self._backend() monkeypatch.setattr( manager.plugin_manager, 'get_analysis_provider', lambda component: backend if component == 'asr' else None, ) assert asr.get_asr_backend() is backend + @pytest.mark.parametrize( + 'missing', ['load_whisper_model', 'transcribe', 'is_loaded', 'unload'] + ) + def test_incomplete_override_falls_back_to_builtin(self, asr, monkeypatch, caplog, missing): + backend = self._backend(**{missing: None}) + monkeypatch.setattr( + manager.plugin_manager, 'get_analysis_provider', lambda component: backend + ) + stub = self._stub_builtin(monkeypatch) + assert asr.get_asr_backend() is stub + assert missing in caplog.text + + def test_non_callable_attribute_is_not_enough(self, asr, monkeypatch): + backend = self._backend(is_loaded=True) + monkeypatch.setattr( + manager.plugin_manager, 'get_analysis_provider', lambda component: backend + ) + stub = self._stub_builtin(monkeypatch) + assert asr.get_asr_backend() is stub + def test_falls_back_to_builtin_when_no_override(self, asr, monkeypatch): monkeypatch.setattr( manager.plugin_manager, 'get_analysis_provider', lambda component: None ) - stub = types.ModuleType('lyrics.whisper_onnx') - monkeypatch.setitem(sys.modules, 'lyrics.whisper_onnx', stub) + stub = self._stub_builtin(monkeypatch) assert asr.get_asr_backend() is stub def test_manager_error_falls_back_to_builtin(self, asr, monkeypatch): @@ -232,6 +487,5 @@ def boom(component): raise RuntimeError('plugin manager exploded') monkeypatch.setattr(manager.plugin_manager, 'get_analysis_provider', boom) - stub = types.ModuleType('lyrics.whisper_onnx') - monkeypatch.setitem(sys.modules, 'lyrics.whisper_onnx', stub) + stub = self._stub_builtin(monkeypatch) assert asr.get_asr_backend() is stub From 31fc89e834aeec56f28f36873b3182039c53da03 Mon Sep 17 00:00:00 2001 From: Schaka <2223171+Schaka@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:53:03 +0200 Subject: [PATCH 3/5] [Plugin] Rework docs and failure points for plugin system --- .gitignore | 3 - docs/PLUGIN.md | 33 ++++++++++- lyrics/_asr_backend.py | 6 ++ lyrics/lyrics_transcriber.py | 45 ++++++++++++--- plugin/api.py | 10 +++- plugin/manager.py | 18 +++++- test/unit/test_lyrics_transcriber.py | 74 ++++++++++++++++++++++++- test/unit/test_plugin_analysis_seams.py | 45 +++++++++++++++ 8 files changed, 215 insertions(+), 19 deletions(-) diff --git a/.gitignore b/.gitignore index ff93b452d..612f23436 100644 --- a/.gitignore +++ b/.gitignore @@ -29,8 +29,6 @@ env/ # IDE and editor configuration .vscode/ .idea/ -.tokensave -.claude/.headroom_wrap_marker.json # macOS Finder / metadata files .DS_Store @@ -78,4 +76,3 @@ analysis.md .claude/ lyrics_model_gte_vnni.tar.gz native-build/windows/vendor/redis/amd64/redis-server.exe -.venv-test \ No newline at end of file diff --git a/docs/PLUGIN.md b/docs/PLUGIN.md index 4fb7035fd..e46ea5648 100644 --- a/docs/PLUGIN.md +++ b/docs/PLUGIN.md @@ -470,11 +470,13 @@ And the methods on the `ctx` object in `register(ctx)`: | `on_install(func)` | Run once at install and on every update. Gets the database connection. | | `on_flask_start(func)` / `on_worker_start(func)` | Run at every start of that container, after the plugin loads. | | `register_onnx_provider(name, options, position, only_models=None, exclude_models=None, needs_static_shapes=False)` | Advanced: offer an extra ONNX Runtime execution provider (for example a GPU) for analysis. See "Offer an ONNX execution provider" below. | -| `register_analysis_provider(component, factory, cache=True)` | Advanced: replace a whole analysis component with a plugin-supplied implementation. `component` is currently `asr` (the Whisper backend); `factory` is the replacement module/object (or a zero-arg callable returning one) matching the built-in surface `load_whisper_model`/`transcribe`/`is_loaded`/`unload`. Consulted before the built-in; an unknown `component` or a backend missing part of that surface is ignored with a warning in the logs, and the built-in is used. `cache=True` (the default) resolves a callable `factory` once and reuses the result, like the built-in modules, which stay loaded for a whole album; pass `cache=False` to be called for every use, and then unload what you hand out yourself. | +| `register_analysis_provider(component, factory, cache=True)` | Advanced: replace a whole analysis component with a plugin-supplied implementation. `component` is currently `asr` (the Whisper backend); `factory` is the replacement module/object (or a zero-arg callable returning one) matching the built-in surface `load_whisper_model`/`transcribe`/`is_loaded`/`unload`. See "Replace an analysis component" below for what each method must return. Consulted before the built-in; an unknown `component` or a backend missing part of that surface is ignored with a warning in the logs, and the built-in is used. `cache=True` (the default) resolves a callable `factory` once and reuses the result, like the built-in modules, which stay loaded for a whole album; pass `cache=False` to be called for every use, and then unload what you hand out yourself. | ### Offer an ONNX execution provider -`register_onnx_provider` adds your provider to the chain AudioMuse-AI builds for every analysis model. `position` is `before_cpu` (the default) or `before_cuda` when your provider should win over CUDA. Set `needs_static_shapes=True` if your graph compiler cannot handle symbolic dimensions, so core pins them before it builds the session (the CLAP audio model needs this). +`register_onnx_provider` adds your provider to the chain AudioMuse-AI builds for every analysis model. `position` is `before_cpu` (the default) or `before_cuda` when your provider should win over CUDA. Set `needs_static_shapes=True` if your graph compiler cannot handle symbolic dimensions, so core pins them before it builds the session; today that only affects the CLAP audio model, the one analysis model with a symbolic axis. + +Your provider must already exist in the `onnxruntime` build of the image you run, because core only offers providers that `onnxruntime.get_available_providers()` reports. A plugin cannot add one by pip-installing into its own `_lib`: execution providers are compiled into `onnxruntime` itself, so a provider that needs a different build needs a different image. A provider core has already put in the chain for that session (`CUDAExecutionProvider`, or `CoreMLExecutionProvider` on macOS) wins: your entry for that same name is dropped and core's own options are kept, so use `position` to order yourself around them instead of re-registering them. Scope the provider to the models it can actually compile with `only_models`/`exclude_models`. Both take a session label or a list of them; an unknown label is logged as a warning. The labels are: @@ -501,6 +503,33 @@ def register(ctx): ) ``` +### Replace an analysis component + +Sometimes a different execution provider is not enough and the step needs a different library altogether: some execution providers cannot run the ONNX Whisper decoder at all, so a plugin can swap in faster-whisper instead. `register_analysis_provider(component, factory)` hands core a full replacement for one step. The only component today is `asr`, the Whisper backend used by the lyrics pipeline. + +Your replacement must expose all four methods below. Core checks for them when it resolves your plugin and falls back to the built-in backend - with a warning naming what is missing - if any is absent. + +| Method | Must return | +|---|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `load_whisper_model()` | Nothing. Load the model, or raise an exception if it cannot be loaded. | +| `transcribe(wav, sr, language=None)` | A dict: `text` (the transcript, `''` when nothing was recognised), `language` (the detected language code, for example `en`), `avg_logprob` (the mean per-token log probability of the transcript). | +| `is_loaded()` | `True` while your model is in memory. Core uses it to decide whether a memory cleanup is needed after an album. | +| `unload()` | Nothing. Free the model and its memory. | + +`avg_logprob` is the one that quietly matters: core drops a transcript whose confidence is below `LYRICS_ASR_MIN_AVG_LOGPROB` (and below `LYRICS_ASR_NON_ENGLISH_MIN_LOGPROB` for non-English) and treats the song as instrumental. Return the real value if your backend has one. If it does not, leave the key out entirely - core then skips the confidence check instead of reading a missing value as the worst possible score. Do not return a placeholder like `0.0`, which disables the check without saying so. + +```python +def register(ctx): + ctx.register_analysis_provider('asr', _load_backend) + + +def _load_backend(): + # Return None to stand down (no GPU here, say); core logs it and keeps the + # built-in backend. Returning the module keeps it loaded for a whole album. + from . import my_whisper + return my_whisper +``` + ## Who can see and manage plugins AudioMuse-AI keeps a clear line between managing plugins and using them. diff --git a/lyrics/_asr_backend.py b/lyrics/_asr_backend.py index 89e120330..a65ea7ae9 100644 --- a/lyrics/_asr_backend.py +++ b/lyrics/_asr_backend.py @@ -15,6 +15,12 @@ same public surface: ``load_whisper_model()``, ``transcribe(wav, sr, language=None)``, ``is_loaded()`` and ``unload()``. +``transcribe`` must return a dict with ``text`` (the transcript), ``language`` +(the detected language code) and ``avg_logprob`` (the mean per-token log +probability, used to gate transcript quality). A backend that cannot report a +confidence should leave ``avg_logprob`` out entirely: the quality gate is then +skipped instead of reading a value that would drop every transcript. + Main Features: * Consults the loaded plugins for an 'asr' analysis provider before the built-in * Falls back to the built-in whisper_onnx backend when no plugin registered one, diff --git a/lyrics/lyrics_transcriber.py b/lyrics/lyrics_transcriber.py index 25b9d756b..7d7f65a00 100644 --- a/lyrics/lyrics_transcriber.py +++ b/lyrics/lyrics_transcriber.py @@ -833,16 +833,25 @@ def _lyrics_result( } +# Decides whether an ASR transcript is too poor to keep. asr_avg_logprob is None +# when the backend reported no confidence at all (see _asr_confidence): the +# confidence thresholds are then skipped rather than treated as the worst possible +# score, so a plugin backend that omits the key loses no transcript. The language +# checks still apply. def _asr_should_drop( - raw_text: str, whisper_raw_len: int, asr_lang: str, asr_avg_logprob: float + raw_text: str, whisper_raw_len: int, asr_lang: str, asr_avg_logprob: Optional[float] ) -> bool: if not raw_text or whisper_raw_len <= 0: return False - if asr_avg_logprob < ASR_MIN_AVG_LOGPROB: + if asr_avg_logprob is not None and asr_avg_logprob < ASR_MIN_AVG_LOGPROB: return True if asr_lang in _ASR_NULL_LANGS: return True - if asr_lang not in _ASR_ENGLISH_LANGS and asr_avg_logprob < ASR_NON_ENGLISH_MIN_LOGPROB: + if ( + asr_lang not in _ASR_ENGLISH_LANGS + and asr_avg_logprob is not None + and asr_avg_logprob < ASR_NON_ENGLISH_MIN_LOGPROB + ): return True return False @@ -975,17 +984,35 @@ def _alarm_handler(signum, frame): signal.signal(signal.SIGALRM, _old_handler) -def _postprocess_asr(transcription: Dict[str, object]) -> Tuple[str, int, str, float, str]: +# Reads the backend's avg_logprob, or None when it reported none. The built-in +# backend always returns it, but a plugin ASR backend registered with +# register_analysis_provider may not. None means "unknown confidence" and switches +# off the confidence gates in _asr_should_drop; assuming the worst score here would +# silently turn a whole library into instrumentals. +def _asr_confidence(transcription: Dict[str, object]) -> Optional[float]: + value = transcription.get('avg_logprob') + if value is None: + return None + try: + return float(value) + except (TypeError, ValueError): + logger.warning('ASR backend returned a non-numeric avg_logprob %r - ignoring it', value) + return None + + +def _postprocess_asr( + transcription: Dict[str, object] +) -> Tuple[str, int, str, Optional[float], str]: raw_text = _sanitize_lyrics_text((transcription.get('text') or '').strip()) whisper_raw_len = len(raw_text) asr_lang = (transcription.get('language') or '').strip().lower() - asr_avg_logprob = float(transcription.get('avg_logprob', float('-inf'))) + asr_avg_logprob = _asr_confidence(transcription) detected_lang = asr_lang or 'en' logger.info( - 'STEP 5 end: transcript length=%s chars / asr_lang=%r / avg_logprob=%.2f', + 'STEP 5 end: transcript length=%s chars / asr_lang=%r / avg_logprob=%s', len(raw_text), asr_lang, - asr_avg_logprob, + 'n/a' if asr_avg_logprob is None else '%.2f' % asr_avg_logprob, ) logger.info('STEP 5 raw ASR output: %s', raw_text or '') _resolved, _script, _reject = _resolve_lang_and_quality(raw_text, asr_lang) @@ -1152,7 +1179,9 @@ def analyze_lyrics( if _asr_should_drop(raw_text, whisper_raw_len, asr_lang, asr_avg_logprob): logger.info( - 'STEP 7: dropping ASR transcript (lang=%r, logprob=%.2f)', asr_lang, asr_avg_logprob + 'STEP 7: dropping ASR transcript (lang=%r, logprob=%s)', + asr_lang, + 'n/a' if asr_avg_logprob is None else '%.2f' % asr_avg_logprob, ) raw_text = '' logger.info('STEP 7 end: language=%s, kept_text=%s', detected_lang, bool(raw_text)) diff --git a/plugin/api.py b/plugin/api.py index 8b78216fa..c3032c84f 100644 --- a/plugin/api.py +++ b/plugin/api.py @@ -306,9 +306,13 @@ def register_analysis_provider(self, component, factory, cache=True): ``factory`` is the replacement module/object, or a zero-arg callable returning one. It must match the built-in module's public surface; for ``asr`` that is ``load_whisper_model()``, ``transcribe(wav, sr, - language=None)``, ``is_loaded()`` and ``unload()``. Core consults the - registered provider first and falls back to the built-in when no plugin - registered one, or when the replacement is missing part of that surface. + language=None)``, ``is_loaded()`` and ``unload()``, where ``transcribe`` + returns ``{'text': ..., 'language': ..., 'avg_logprob': ...}`` - the last + one gates transcript quality and must be left out, not faked, when the + backend cannot report a confidence. Core consults the registered provider + first and falls back to the built-in when no plugin registered one, when + the factory fails or returns None, or when the replacement is missing part + of that surface. A callable ``factory`` is resolved once and the result reused, matching the built-in modules, which stay loaded for a whole album and are freed at its diff --git a/plugin/manager.py b/plugin/manager.py index a11f42961..c5e746969 100644 --- a/plugin/manager.py +++ b/plugin/manager.py @@ -872,6 +872,9 @@ def get_analysis_provider(self, component): zero-arg callable) and reuses that result unless the plugin registered with ``cache=False``, so a model is not rebuilt on every call. Used by core to let a plugin swap out a whole analysis step such as the ASR/Whisper backend. + + A plugin whose factory raises or returns None is logged by id and skipped, + so core falls back to the built-in component with a trace of why. """ if component in self._analysis_provider_cache: return self._analysis_provider_cache[component] @@ -893,7 +896,20 @@ def get_analysis_provider(self, component): factory = entry['factory'] provider = factory() if callable(factory) else factory except Exception: - logger.exception('Plugin analysis provider for %r failed to resolve', component) + logger.exception( + 'Plugin %s: its analysis provider for %r failed to resolve; ' + 'falling back to the next provider or the built-in one', + plugin_id, component, + ) + continue + if provider is None: + # A factory returning None is a plugin opting out at runtime (no + # GPU found, say). Say so, or the built-in silently stays in use. + logger.warning( + 'Plugin %s: its analysis provider factory for %r returned None; ' + 'falling back to the next provider or the built-in one', + plugin_id, component, + ) continue if entry['cache']: self._analysis_provider_cache[component] = provider diff --git a/test/unit/test_lyrics_transcriber.py b/test/unit/test_lyrics_transcriber.py index 141a37643..fdc357d41 100644 --- a/test/unit/test_lyrics_transcriber.py +++ b/test/unit/test_lyrics_transcriber.py @@ -6,10 +6,11 @@ # the terms of the GNU Affero General Public License v3.0. See the LICENSE file # in the project root or -"""Lyrics axis scoring and text sanitization in lyrics.lyrics_transcriber. +"""Lyrics axis scoring, text sanitization and ASR gating in lyrics.lyrics_transcriber. Covers the pinned 27-label axis column order, per-axis softmax scoring of an -embedding, and the cleanup applied to raw lyric text before embedding. +embedding, the cleanup applied to raw lyric text before embedding, and the +transcript quality gate that reads the ASR backend's reported confidence. Main Features: * axis_columns is a pure, duplicate-free 27-tuple in canonical axis/label order @@ -17,6 +18,9 @@ points at the targeted label * _sanitize_lyrics_text strips control/zero-width chars, HTML, and LRC timestamps and truncates to a word cap; _softmax sums to one and is temperature-monotonic +* _asr_confidence reads avg_logprob as unknown (None) when a backend omits it or + reports it non-numerically, and _asr_should_drop then skips the confidence + gates instead of dropping every transcript """ from __future__ import annotations @@ -258,6 +262,72 @@ def test_drops_empty_lines_after_stripping(self, lt): assert out == 'only content' +class TestAsrConfidence: + """A plugin ASR backend may not report avg_logprob at all - see _asr_backend.""" + + def test_reads_the_reported_value(self, lt): + assert lt._asr_confidence({'avg_logprob': -0.25}) == pytest.approx(-0.25) + + def test_missing_key_is_unknown(self, lt): + assert lt._asr_confidence({'text': 'hi', 'language': 'en'}) is None + + def test_explicit_none_is_unknown(self, lt): + assert lt._asr_confidence({'avg_logprob': None}) is None + + def test_non_numeric_is_unknown_and_warns(self, lt, caplog): + assert lt._asr_confidence({'avg_logprob': 'very good'}) is None + assert 'non-numeric avg_logprob' in caplog.text + + def test_postprocess_keeps_a_transcript_without_confidence(self, lt): + # Long enough to clear the minimum-length quality check on its own. + text = ( + 'I walked alone into the pouring rain and thought about the days ' + 'we spent together, the summer light, the empty street, the sound ' + 'of your name, the songs we sang out loud until the morning came ' + 'and left us nothing but the echo of a promise we could not keep.' + ) + raw_text, raw_len, lang, logprob, detected = lt._postprocess_asr( + {'text': text, 'language': 'en'} + ) + assert raw_text == text + assert raw_len == len(text) + assert lang == 'en' + assert detected == 'en' + assert logprob is None + + +class TestAsrShouldDrop: + GOOD_TEXT = 'a real transcript' + + def _drop(self, lt, lang, logprob, text=None): + text = self.GOOD_TEXT if text is None else text + return lt._asr_should_drop(text, len(text), lang, logprob) + + def test_drops_low_confidence(self, lt): + assert self._drop(lt, 'en', lt.ASR_MIN_AVG_LOGPROB - 0.1) is True + + def test_keeps_good_confidence(self, lt): + assert self._drop(lt, 'en', lt.ASR_MIN_AVG_LOGPROB + 0.1) is False + + def test_unknown_confidence_skips_the_gate(self, lt): + # The whole point: a backend that reports no confidence must not lose + # every transcript to the quality gate. + assert self._drop(lt, 'en', None) is False + + def test_unknown_confidence_skips_the_non_english_gate(self, lt): + assert self._drop(lt, 'de', None) is False + + def test_drops_low_confidence_non_english(self, lt): + logprob = lt.ASR_NON_ENGLISH_MIN_LOGPROB - 0.1 + assert self._drop(lt, 'de', logprob) is True + + def test_null_language_still_drops_without_confidence(self, lt): + assert self._drop(lt, 'nospeech', None) is True + + def test_empty_text_is_never_dropped_here(self, lt): + assert self._drop(lt, 'en', float('-inf'), text='') is False + + class TestSoftmax: def test_sums_to_one(self, lt): v = np.array([1.0, 2.0, 3.0, 4.0], dtype=np.float32) diff --git a/test/unit/test_plugin_analysis_seams.py b/test/unit/test_plugin_analysis_seams.py index cd7e8c89e..826c44ffd 100644 --- a/test/unit/test_plugin_analysis_seams.py +++ b/test/unit/test_plugin_analysis_seams.py @@ -24,6 +24,8 @@ * register_analysis_provider/get_analysis_provider swap a whole component (asr), resolving module objects and zero-arg factories once (unless cache=False), rejecting unknown components and swallowing broken plugins +* a factory that raises or returns None is logged with its plugin id and skipped, + so the fallback to the built-in component is visible and not cached * lyrics.get_asr_backend prefers a registered override, and falls back to the built-in when there is none or the override misses part of the whisper surface """ @@ -421,6 +423,49 @@ def boom(): mgr.records = {'p': _record('p', analysis_providers={'asr': boom})} assert mgr.get_analysis_provider('asr') is None + def test_broken_factory_is_logged_with_the_plugin_id(self, caplog): + mgr = manager.PluginManager() + + def boom(): + raise RuntimeError('backend import failed') + + mgr.records = {'noisy_plugin': _record('noisy_plugin', analysis_providers={'asr': boom})} + mgr.get_analysis_provider('asr') + # Without the id the admin cannot tell which plugin to fix or remove. + assert 'noisy_plugin' in caplog.text + + def test_factory_returning_none_is_logged(self, caplog): + mgr = manager.PluginManager() + mgr.records = {'quiet_plugin': _record( + 'quiet_plugin', analysis_providers={'asr': lambda: None}, + )} + assert mgr.get_analysis_provider('asr') is None + assert 'quiet_plugin' in caplog.text + assert 'returned None' in caplog.text + + def test_factory_returning_none_lets_the_next_plugin_provide(self, caplog): + mgr = manager.PluginManager() + backend = object() + mgr.records = { + 'a': _record('a', analysis_providers={'asr': lambda: None}), + 'b': _record('b', analysis_providers={'asr': backend}), + } + assert mgr.get_analysis_provider('asr') is backend + assert 'returned None' in caplog.text + + def test_none_result_is_not_cached(self): + mgr = manager.PluginManager() + backend = object() + results = [None, backend] + + def factory(): + return results.pop(0) + + mgr.records = {'p': _record('p', analysis_providers={'asr': factory})} + assert mgr.get_analysis_provider('asr') is None + # None means "not this time", so a later call asks the plugin again. + assert mgr.get_analysis_provider('asr') is backend + class TestGetAsrBackend: @pytest.fixture From 58a0b8359215825022627b3d82eca92bedcfaa5c Mon Sep 17 00:00:00 2001 From: neptunehub Date: Wed, 29 Jul 2026 12:29:31 +0200 Subject: [PATCH 4/5] Harden plugin ASR seam: treat NaN/overflow avg_logprob as unknown confidence, name the owning plugin in fallback warnings, fix PLUGIN.md static-shape claim --- docs/PLUGIN.md | 6 +-- lyrics/_asr_backend.py | 15 +++++- lyrics/lyrics_transcriber.py | 9 +++- plugin/manager.py | 6 +++ test/unit/test_lyrics_transcriber.py | 7 +++ test/unit/test_plugin_analysis_seams.py | 61 +++++++++++++++++++++++++ 6 files changed, 97 insertions(+), 7 deletions(-) diff --git a/docs/PLUGIN.md b/docs/PLUGIN.md index e46ea5648..d3077ee98 100644 --- a/docs/PLUGIN.md +++ b/docs/PLUGIN.md @@ -474,7 +474,7 @@ And the methods on the `ctx` object in `register(ctx)`: ### Offer an ONNX execution provider -`register_onnx_provider` adds your provider to the chain AudioMuse-AI builds for every analysis model. `position` is `before_cpu` (the default) or `before_cuda` when your provider should win over CUDA. Set `needs_static_shapes=True` if your graph compiler cannot handle symbolic dimensions, so core pins them before it builds the session; today that only affects the CLAP audio model, the one analysis model with a symbolic axis. +`register_onnx_provider` adds your provider to the chain AudioMuse-AI builds for every analysis model. `position` is `before_cpu` (the default) or `before_cuda` when your provider should win over CUDA. Set `needs_static_shapes=True` if your graph compiler cannot handle symbolic dimensions, so core pins them before it builds the session; today core pins them only for the CLAP audio model. Other models (the GTE embedder and the Whisper decoder, for example) also carry symbolic axes but are never pinned, so a provider that cannot compile them must be scoped away from those models with `only_models`/`exclude_models`. Your provider must already exist in the `onnxruntime` build of the image you run, because core only offers providers that `onnxruntime.get_available_providers()` reports. A plugin cannot add one by pip-installing into its own `_lib`: execution providers are compiled into `onnxruntime` itself, so a provider that needs a different build needs a different image. A provider core has already put in the chain for that session (`CUDAExecutionProvider`, or `CoreMLExecutionProvider` on macOS) wins: your entry for that same name is dropped and core's own options are kept, so use `position` to order yourself around them instead of re-registering them. @@ -507,7 +507,7 @@ def register(ctx): Sometimes a different execution provider is not enough and the step needs a different library altogether: some execution providers cannot run the ONNX Whisper decoder at all, so a plugin can swap in faster-whisper instead. `register_analysis_provider(component, factory)` hands core a full replacement for one step. The only component today is `asr`, the Whisper backend used by the lyrics pipeline. -Your replacement must expose all four methods below. Core checks for them when it resolves your plugin and falls back to the built-in backend - with a warning naming what is missing - if any is absent. +Your replacement must expose all four methods below. Core checks for them when it resolves your plugin and falls back to the built-in backend - with a warning naming your plugin and what is missing - if any is absent. | Method | Must return | |---|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| @@ -516,7 +516,7 @@ Your replacement must expose all four methods below. Core checks for them when i | `is_loaded()` | `True` while your model is in memory. Core uses it to decide whether a memory cleanup is needed after an album. | | `unload()` | Nothing. Free the model and its memory. | -`avg_logprob` is the one that quietly matters: core drops a transcript whose confidence is below `LYRICS_ASR_MIN_AVG_LOGPROB` (and below `LYRICS_ASR_NON_ENGLISH_MIN_LOGPROB` for non-English) and treats the song as instrumental. Return the real value if your backend has one. If it does not, leave the key out entirely - core then skips the confidence check instead of reading a missing value as the worst possible score. Do not return a placeholder like `0.0`, which disables the check without saying so. +`avg_logprob` is the one that quietly matters: core drops a transcript whose confidence is below `LYRICS_ASR_MIN_AVG_LOGPROB` (and below `LYRICS_ASR_NON_ENGLISH_MIN_LOGPROB` for non-English) and treats the song as instrumental. Return the real value if your backend has one. If it does not, leave the key out entirely - core then skips the confidence check instead of reading a missing value as the worst possible score. Do not return a placeholder like `0.0`, which disables the check without saying so. An explicit `None` or a NaN is treated like a missing key. ```python def register(ctx): diff --git a/lyrics/_asr_backend.py b/lyrics/_asr_backend.py index a65ea7ae9..5564aa15c 100644 --- a/lyrics/_asr_backend.py +++ b/lyrics/_asr_backend.py @@ -25,6 +25,8 @@ * Consults the loaded plugins for an 'asr' analysis provider before the built-in * Falls back to the built-in whisper_onnx backend when no plugin registered one, or when the replacement is missing part of the required surface +* Rejection of an incomplete replacement is warned on every resolution, naming + the owning plugin so the user knows which one to fix or remove * Swallows any plugin-resolution error so a broken plugin never breaks lyrics """ @@ -55,6 +57,15 @@ def _plugin_asr_backend(): return None +def _provider_owner(): + try: + from plugin.manager import plugin_manager + + return plugin_manager.analysis_provider_owner('asr') + except Exception: + return None + + # Rejects a backend that cannot stand in for whisper_onnx. A missing method would # surface far from here: a broken is_loaded makes is_lyrics_loaded() report True # forever, so every album pays a full memory cleanup for nothing, and a broken @@ -66,7 +77,7 @@ def _has_required_surface(backend): if not missing: return True logger.warning( - 'Plugin ASR backend %r is missing %s; using the built-in whisper instead', - backend, ', '.join(missing), + 'Plugin %s: its ASR backend %r is missing %s; using the built-in whisper instead', + _provider_owner() or 'unknown', backend, ', '.join(missing), ) return False diff --git a/lyrics/lyrics_transcriber.py b/lyrics/lyrics_transcriber.py index 7d7f65a00..c58af3f17 100644 --- a/lyrics/lyrics_transcriber.py +++ b/lyrics/lyrics_transcriber.py @@ -28,6 +28,7 @@ from __future__ import annotations import logging +import math import os import re import signal @@ -994,10 +995,14 @@ def _asr_confidence(transcription: Dict[str, object]) -> Optional[float]: if value is None: return None try: - return float(value) - except (TypeError, ValueError): + confidence = float(value) + except (TypeError, ValueError, OverflowError): logger.warning('ASR backend returned a non-numeric avg_logprob %r - ignoring it', value) return None + if math.isnan(confidence): + logger.warning('ASR backend returned a NaN avg_logprob - treating confidence as unknown') + return None + return confidence def _postprocess_asr( diff --git a/plugin/manager.py b/plugin/manager.py index c5e746969..979a834e9 100644 --- a/plugin/manager.py +++ b/plugin/manager.py @@ -280,6 +280,7 @@ def __init__(self): self._runtime_dirty = False self.last_pip_error = None self._analysis_provider_cache = {} + self._analysis_provider_owners = {} def enabled(self): return bool(config.PLUGINS_ENABLED) @@ -318,6 +319,7 @@ def _runs_here(self, record, role): def sync(self, conn=None, role=None): self._analysis_provider_cache = {} + self._analysis_provider_owners = {} if not self.enabled(): self.records = {} return @@ -911,11 +913,15 @@ def get_analysis_provider(self, component): plugin_id, component, ) continue + self._analysis_provider_owners[component] = plugin_id if entry['cache']: self._analysis_provider_cache[component] = provider return provider return None + def analysis_provider_owner(self, component): + return self._analysis_provider_owners.get(component) + def song_analyzed_hooks(self): hooks = [] for record in self.records.values(): diff --git a/test/unit/test_lyrics_transcriber.py b/test/unit/test_lyrics_transcriber.py index fdc357d41..142696a25 100644 --- a/test/unit/test_lyrics_transcriber.py +++ b/test/unit/test_lyrics_transcriber.py @@ -278,6 +278,13 @@ def test_non_numeric_is_unknown_and_warns(self, lt, caplog): assert lt._asr_confidence({'avg_logprob': 'very good'}) is None assert 'non-numeric avg_logprob' in caplog.text + def test_nan_is_unknown_and_warns_instead_of_disabling_the_gate(self, lt, caplog): + assert lt._asr_confidence({'avg_logprob': float('nan')}) is None + assert 'NaN avg_logprob' in caplog.text + + def test_overflowing_value_is_unknown_not_fatal(self, lt): + assert lt._asr_confidence({'avg_logprob': 10 ** 400}) is None + def test_postprocess_keeps_a_transcript_without_confidence(self, lt): # Long enough to clear the minimum-length quality check on its own. text = ( diff --git a/test/unit/test_plugin_analysis_seams.py b/test/unit/test_plugin_analysis_seams.py index 826c44ffd..ae6c06d63 100644 --- a/test/unit/test_plugin_analysis_seams.py +++ b/test/unit/test_plugin_analysis_seams.py @@ -466,6 +466,44 @@ def factory(): # None means "not this time", so a later call asks the plugin again. assert mgr.get_analysis_provider('asr') is backend + def test_broken_factory_reinvoked_and_logged_on_every_call(self, caplog): + mgr = manager.PluginManager() + calls = [] + + def boom(): + calls.append(1) + raise RuntimeError('backend import failed') + + mgr.records = {'p': _record('p', analysis_providers={'asr': boom})} + assert mgr.get_analysis_provider('asr') is None + assert mgr.get_analysis_provider('asr') is None + assert len(calls) == 2 + assert len([r for r in caplog.records if 'failed to resolve' in r.getMessage()]) == 2 + + def test_none_factory_logged_on_every_call(self, caplog): + mgr = manager.PluginManager() + mgr.records = {'p': _record('p', analysis_providers={'asr': lambda: None})} + assert mgr.get_analysis_provider('asr') is None + assert mgr.get_analysis_provider('asr') is None + assert len([r for r in caplog.records if 'returned None' in r.getMessage()]) == 2 + + def test_sync_resets_the_owner(self, monkeypatch): + mgr = manager.PluginManager() + mgr._analysis_provider_owners = {'asr': 'p'} + monkeypatch.setattr(mgr, 'enabled', lambda: False) + mgr.sync() + assert mgr._analysis_provider_owners == {} + + def test_owner_of_the_resolved_provider_is_exposed(self): + mgr = manager.PluginManager() + mgr.records = {'gpu_plugin': _record('gpu_plugin', analysis_providers={'asr': object()})} + assert mgr.get_analysis_provider('asr') is not None + assert mgr.analysis_provider_owner('asr') == 'gpu_plugin' + + def test_owner_is_none_when_nothing_resolved(self): + mgr = manager.PluginManager() + assert mgr.analysis_provider_owner('asr') is None + class TestGetAsrBackend: @pytest.fixture @@ -512,6 +550,29 @@ def test_incomplete_override_falls_back_to_builtin(self, asr, monkeypatch, caplo assert asr.get_asr_backend() is stub assert missing in caplog.text + def test_incomplete_override_warning_names_the_owning_plugin(self, asr, monkeypatch, caplog): + backend = self._backend(unload=None) + monkeypatch.setattr( + manager.plugin_manager, 'get_analysis_provider', lambda component: backend + ) + monkeypatch.setattr( + manager.plugin_manager, 'analysis_provider_owner', lambda component: 'gpu_plugin' + ) + stub = self._stub_builtin(monkeypatch) + assert asr.get_asr_backend() is stub + assert 'gpu_plugin' in caplog.text + assert 'unload' in caplog.text + + def test_incomplete_override_warns_on_every_call(self, asr, monkeypatch, caplog): + backend = self._backend(unload=None) + monkeypatch.setattr( + manager.plugin_manager, 'get_analysis_provider', lambda component: backend + ) + stub = self._stub_builtin(monkeypatch) + assert asr.get_asr_backend() is stub + assert asr.get_asr_backend() is stub + assert len([r for r in caplog.records if 'is missing' in r.getMessage()]) == 2 + def test_non_callable_attribute_is_not_enough(self, asr, monkeypatch): backend = self._backend(is_loaded=True) monkeypatch.setattr( From 822d847d37eb709d29a16ce6533fa4ad009984d8 Mon Sep 17 00:00:00 2001 From: neptunehub Date: Wed, 29 Jul 2026 13:57:36 +0200 Subject: [PATCH 5/5] Truthful multi-provider ASR logs, pin +/-inf avg_logprob semantics, warn on impossible confidence, None-init and stale-owner cleanup --- docs/PLUGIN.md | 2 +- lyrics/lyrics_transcriber.py | 8 +++++++- plugin/manager.py | 10 ++++++++-- test/unit/test_lyrics_transcriber.py | 18 ++++++++++++++++++ test/unit/test_plugin_analysis_seams.py | 24 ++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/docs/PLUGIN.md b/docs/PLUGIN.md index d3077ee98..c64ea8b24 100644 --- a/docs/PLUGIN.md +++ b/docs/PLUGIN.md @@ -516,7 +516,7 @@ Your replacement must expose all four methods below. Core checks for them when i | `is_loaded()` | `True` while your model is in memory. Core uses it to decide whether a memory cleanup is needed after an album. | | `unload()` | Nothing. Free the model and its memory. | -`avg_logprob` is the one that quietly matters: core drops a transcript whose confidence is below `LYRICS_ASR_MIN_AVG_LOGPROB` (and below `LYRICS_ASR_NON_ENGLISH_MIN_LOGPROB` for non-English) and treats the song as instrumental. Return the real value if your backend has one. If it does not, leave the key out entirely - core then skips the confidence check instead of reading a missing value as the worst possible score. Do not return a placeholder like `0.0`, which disables the check without saying so. An explicit `None` or a NaN is treated like a missing key. +`avg_logprob` is the one that quietly matters: core drops a transcript whose confidence is below `LYRICS_ASR_MIN_AVG_LOGPROB` (and below `LYRICS_ASR_NON_ENGLISH_MIN_LOGPROB` for non-English) and treats the song as instrumental. Return the real value if your backend has one. If it does not, leave the key out entirely - core then skips the confidence check instead of reading a missing value as the worst possible score. Do not return a placeholder like `0.0`, which disables the check without saying so. An explicit `None` or a NaN is treated like a missing key. Infinities are not: they pass through as real values, so `-inf` - the built-in backend's own worst-confidence value - always drops the transcript. A value of `0.0` or above is kept but flagged in the log, because no real mean log-probability is positive. ```python def register(ctx): diff --git a/lyrics/lyrics_transcriber.py b/lyrics/lyrics_transcriber.py index c58af3f17..1438a8a96 100644 --- a/lyrics/lyrics_transcriber.py +++ b/lyrics/lyrics_transcriber.py @@ -1002,6 +1002,12 @@ def _asr_confidence(transcription: Dict[str, object]) -> Optional[float]: if math.isnan(confidence): logger.warning('ASR backend returned a NaN avg_logprob - treating confidence as unknown') return None + if confidence >= 0: + logger.warning( + 'ASR backend returned avg_logprob %s but a real mean log-probability is negative - ' + 'the confidence gate cannot drop this transcript', + confidence, + ) return confidence @@ -1150,7 +1156,7 @@ def analyze_lyrics( used_seconds = 0.0 detected_lang = 'en' asr_lang = 'en' - asr_avg_logprob = 0.0 + asr_avg_logprob = None whisper_raw_len = 0 normalized_moods, vocal_prior = _compute_vocal_prior(top_moods) diff --git a/plugin/manager.py b/plugin/manager.py index 979a834e9..fb542bea8 100644 --- a/plugin/manager.py +++ b/plugin/manager.py @@ -887,8 +887,8 @@ def get_analysis_provider(self, component): ] if len(owners) > 1: logger.warning( - 'Plugins %s all provide the analysis component %r; using %s', - ', '.join(owners), component, owners[0], + 'Plugins %s all provide the analysis component %r; trying them in that order', + ', '.join(owners), component, ) for plugin_id in owners: entry = _analysis_provider_entry( @@ -914,9 +914,15 @@ def get_analysis_provider(self, component): ) continue self._analysis_provider_owners[component] = plugin_id + if len(owners) > 1: + logger.warning( + 'Plugin %s: its analysis provider for %r is the one in use', + plugin_id, component, + ) if entry['cache']: self._analysis_provider_cache[component] = provider return provider + self._analysis_provider_owners.pop(component, None) return None def analysis_provider_owner(self, component): diff --git a/test/unit/test_lyrics_transcriber.py b/test/unit/test_lyrics_transcriber.py index 142696a25..adbcb9cd9 100644 --- a/test/unit/test_lyrics_transcriber.py +++ b/test/unit/test_lyrics_transcriber.py @@ -285,6 +285,21 @@ def test_nan_is_unknown_and_warns_instead_of_disabling_the_gate(self, lt, caplog def test_overflowing_value_is_unknown_not_fatal(self, lt): assert lt._asr_confidence({'avg_logprob': 10 ** 400}) is None + def test_neg_inf_is_the_builtin_worst_confidence_sentinel_and_stays_real(self, lt): + assert lt._asr_confidence({'avg_logprob': float('-inf')}) == float('-inf') + + def test_pos_inf_stays_real_but_warns_as_impossible(self, lt, caplog): + assert lt._asr_confidence({'avg_logprob': float('inf')}) == float('inf') + assert 'cannot drop this transcript' in caplog.text + + def test_zero_the_documented_forbidden_placeholder_is_kept_but_warned(self, lt, caplog): + assert lt._asr_confidence({'avg_logprob': 0.0}) == 0.0 + assert 'cannot drop this transcript' in caplog.text + + def test_good_negative_confidence_does_not_warn(self, lt, caplog): + assert lt._asr_confidence({'avg_logprob': -0.3}) == pytest.approx(-0.3) + assert 'cannot drop this transcript' not in caplog.text + def test_postprocess_keeps_a_transcript_without_confidence(self, lt): # Long enough to clear the minimum-length quality check on its own. text = ( @@ -316,6 +331,9 @@ def test_drops_low_confidence(self, lt): def test_keeps_good_confidence(self, lt): assert self._drop(lt, 'en', lt.ASR_MIN_AVG_LOGPROB + 0.1) is False + def test_neg_inf_confidence_still_drops_it_is_not_an_unknown(self, lt): + assert self._drop(lt, 'en', float('-inf')) is True + def test_unknown_confidence_skips_the_gate(self, lt): # The whole point: a backend that reports no confidence must not lose # every transcript to the quality gate. diff --git a/test/unit/test_plugin_analysis_seams.py b/test/unit/test_plugin_analysis_seams.py index ae6c06d63..652e6519c 100644 --- a/test/unit/test_plugin_analysis_seams.py +++ b/test/unit/test_plugin_analysis_seams.py @@ -452,6 +452,14 @@ def test_factory_returning_none_lets_the_next_plugin_provide(self, caplog): } assert mgr.get_analysis_provider('asr') is backend assert 'returned None' in caplog.text + winner_logs = [r.getMessage() for r in caplog.records if 'the one in use' in r.getMessage()] + assert winner_logs == ["Plugin b: its analysis provider for 'asr' is the one in use"] + + def test_single_provider_resolution_does_not_log_a_winner(self, caplog): + mgr = manager.PluginManager() + mgr.records = {'p': _record('p', analysis_providers={'asr': object()})} + assert mgr.get_analysis_provider('asr') is not None + assert 'the one in use' not in caplog.text def test_none_result_is_not_cached(self): mgr = manager.PluginManager() @@ -504,6 +512,22 @@ def test_owner_is_none_when_nothing_resolved(self): mgr = manager.PluginManager() assert mgr.analysis_provider_owner('asr') is None + def test_owner_cleared_when_an_uncached_provider_stops_resolving(self): + mgr = manager.PluginManager() + backend = object() + results = [backend, None] + + def factory(): + return results.pop(0) + + mgr.records = {'p': _record('p', analysis_providers={ + 'asr': {'factory': factory, 'cache': False}, + })} + assert mgr.get_analysis_provider('asr') is backend + assert mgr.analysis_provider_owner('asr') == 'p' + assert mgr.get_analysis_provider('asr') is None + assert mgr.analysis_provider_owner('asr') is None + class TestGetAsrBackend: @pytest.fixture