diff --git a/docs/traceability/analysis-deprecation-warning-policy.md b/docs/traceability/analysis-deprecation-warning-policy.md new file mode 100644 index 000000000..68588ec51 --- /dev/null +++ b/docs/traceability/analysis-deprecation-warning-policy.md @@ -0,0 +1,95 @@ +# Analysis deprecation-warning policy + +Status: Proposed + +## Problem + +Protected `develop@314ddeae7b775a4957594b599358c8255617eb2e` configured analysis-engine pytest with a repository-wide `ignore::DeprecationWarning`. The same protected tree also suppressed every `DeprecationWarning` attributed to `^audioread` around three production decode paths. Those rules made it impossible to distinguish resolved compatibility churn from a newly introduced deprecated API. + +The first repair removed those deprecation suppressions but left module-wide `FutureWarning` ignores for `^audioread` in Temporal Analysis and Separation. Those filters had the same structural defect: category + module with no exact message, upstream cause, or removal condition. They could therefore hide a future audioread compatibility change unrelated to the warning that originally motivated the filter. + +A second policy gap remained after those source filters were removed: pytest failed closed on `DeprecationWarning`, but not on `FutureWarning`. Python distinguishes the two by intended audience, not by whether the warning can precede a breaking API change. Leaving `FutureWarning` at default handling would therefore let an end-user-facing compatibility warning appear in CI without making the warning audit gate fail. + +The three audio loaders also retained message/module-scoped librosa/Numba ignores. Those exceptions still converted dependency warnings into silence before pytest could observe them, while the branch had no exact-head warning inventory proving that either exception was currently necessary. Keeping a suppression first and asking CI to inventory warnings later is circular: the gate cannot report a warning that production code has already discarded. + +The analysis lock resolves `audioread==3.1.0`. Upstream's 3.1.0 history records Python 3.12/3.13 support and replacement of the deprecated `aifc` and `sunau` standard-library modules. librosa 0.11.0 separately documents audioread support itself as deprecated and planned for removal in librosa 1.0. A warning ignore without current execution evidence can outlive the warning that originally motivated it and hide a different warning later. + +References: + +- Python warnings control: https://docs.python.org/3.13/library/warnings.html +- audioread 3.1.0 README/version history: https://github.com/beetbox/audioread/blob/v3.1.0/README.rst +- librosa 0.11.0 advanced I/O documentation: https://librosa.org/doc/0.11.0/ioformats.html + +## Constraints + +- Do not change audio decode parameters, supported formats, dependencies, lockfiles, or MIR behavior merely to make warning output quiet. +- Do not turn a warning failure into success by restoring a global ignore, broad module ignore, test exclusion, `noqa`, or gate change. +- A temporary third-party warning suppression requires an observed exact-head warning plus exact category/message/module, upstream cause, and a concrete removal condition. It must not be pre-installed before that evidence exists. +- A Draft branch is not protected product truth. Fresh exact-head tests and cross-platform CI must expose warnings hidden by the previous policy. + +## Alternatives considered + +### Keep the global pytest ignore + +Rejected. It makes all `DeprecationWarning` instances invisible, including BandScope-owned deprecated calls and new dependency regressions. + +### Revert to Python's default warning behavior + +Rejected for CI. Python's warnings filter can ignore or only display categories depending on defaults and location. The analysis acceptance gate needs an observed `DeprecationWarning` or `FutureWarning` to fail rather than merely appear in logs. + +### Keep broad `^audioread` warning filters around decode + +Rejected. A category + module filter cannot distinguish one historical compatibility warning from a future unrelated warning in the same package. This applies to both `DeprecationWarning` and `FutureWarning`. + +### Keep the existing librosa/Numba warning exceptions until CI proves they are stale + +Rejected. Runtime suppression prevents CI from observing the very warning needed to justify, repair, or remove the exception. Without current exact-head evidence, a pre-existing ignore has no testable necessity or removal trigger. + +### Fail tests on compatibility warnings and expose loader warnings to the gate + +Selected. Unowned `DeprecationWarning` and `FutureWarning` instances become test failures, and the three production audio loaders no longer discard warnings before pytest can classify them. If exact-head execution later proves that an upstream-only warning cannot yet be repaired, a narrow temporary exception can be reintroduced only with the observed warning identity, upstream evidence, regression coverage, and removal condition. + +## Implementation evidence + +- `c9a996912e73c2a5aa0046f8afec5518c892b2e6`: source-level RED requiring pytest not to hide all deprecations. +- `5093dce9425d94b5dc38b54273e635a42d0648aa`: switch pytest to `error::DeprecationWarning`. +- `7885f89c59d3e4b2296efc2fee0379bb09af33f7`: source-level RED rejecting blanket audioread deprecation filters in Temporal Analysis, Transcription, and Separation. +- `ff57338040adfd42404c8066f2ac932aa3e94e82`, `594ea6de4ebb776e8cd7d41299339299a90d7f1a`, `434baa1bd3e27b8a49d8751334a27ffacb51f830`: remove those production deprecation suppressions without changing decode arguments or dependencies. +- `0364d68200a9822f3df2d722164ab3f1a6d89c07`: preserve the existing `pyproject.toml` final newline after the policy edit. +- `fec1d8e9c4f320dd38a803b81f1310eb1238e563`: extend the source-level policy RED so category + `^audioread` filters without an exact message are rejected for both `DeprecationWarning` and `FutureWarning`. +- `e5197fb25bbb2c5f9ae412e24a14d69f3071753f` and `072ebf5714489d0f840d02a12e0344d37dfe35f4`: remove the remaining blanket audioread `FutureWarning` suppressions from Temporal Analysis and Separation. Transcription had no remaining `FutureWarning` suppression. +- `10b5d28ced09c64f148e3b98b7d924357bc20cd4`: policy RED requiring pytest to fail on unowned `FutureWarning` and forbidding a global future-warning ignore. +- `21bbeedcf913d2bbf46ba6ac9a4d5797469ce565`: add `error::FutureWarning` beside the existing deprecation error policy without changing dependencies or decode behavior. +- `fa7adf40294f25325b60cb5abb7cd8f073cb070b`: source-policy RED requiring the three production audio loaders not to install runtime `ignore` filters before warning evidence exists. +- `505d3e14b4559949b6c9cef97bb5b472ea298ae0`: remove Temporal Analysis' remaining librosa/Numba runtime suppression and its warning plumbing. +- `3d94c2f5c5d57d91ded1bba211429e2af71b1537`: remove Separation's remaining librosa/Numba runtime suppression and inherited warning-filter dependency on Temporal Analysis. + +These commits prove the policy/source change only. They do not prove that the complete analysis suite is warning-clean; that requires terminal exact-head execution after this document is committed. + +## Risks and effects + +A previously non-fatal dependency `FutureWarning` or hidden `DeprecationWarning` may now fail CI. That is an intended diagnostic outcome, not a compatibility claim. The causal response is to inspect the originating package/module/call path, migrate BandScope-owned use, upgrade or change a dependency call path where compatible, or document an exact temporary third-party exception with a removal condition. + +Removing the loader suppressions does not change sample rate, mono conversion, duration limits, supported formats, or the librosa call path. It only restores warning observability. The change does not itself remove librosa's deprecated audioread fallback; format-support changes require separate buyer-facing evidence because forcing a new decoder path can alter which real audio files BandScope accepts. + +## Follow-up + +1. Run the full analysis suite with the fail-on-deprecation/future-warning policy and no loader-side ignores. +2. Record every distinct warning by category, message, originating module/package, and call path. +3. Repair owned deprecated calls and rerun the focused/full suites. +4. For an upstream-only warning that cannot yet be repaired, add no suppression until its exact identity, upstream cause, regression coverage, and removal condition are documented. +5. Keep the PR Draft until exact-head repository/central gates and an independent non-author review are complete. + +## Security Notes + +### Trust boundary + +Dependency/runtime warning output is diagnostic input to the analysis acceptance gate. A suppression rule changes which dependency and owned-code regressions become review-visible evidence. + +### Mitigations + +The pytest policy fails on both `DeprecationWarning` and `FutureWarning`, and the source-policy regression prevents the three production audio loaders from discarding runtime warnings before the gate observes them. Any future exception requires evidence rather than inheriting a legacy ignore. + +### Remaining risk + +This policy does not make third-party dependency behavior safe by itself and does not yet prove zero warnings on every supported Windows/macOS runtime. Exact-head hosted execution remains required. diff --git a/services/analysis-engine/pyproject.toml b/services/analysis-engine/pyproject.toml index fb8f7f062..f4e63f100 100644 --- a/services/analysis-engine/pyproject.toml +++ b/services/analysis-engine/pyproject.toml @@ -33,7 +33,8 @@ packages = ["src/bandscope_analysis"] testpaths = ["tests"] pythonpath = ["src"] filterwarnings = [ - "ignore::DeprecationWarning", + "error::DeprecationWarning", + "error::FutureWarning", ] [tool.coverage.run] diff --git a/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py b/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py index c36e0f1fc..603f12fb6 100644 --- a/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py +++ b/services/analysis-engine/src/bandscope_analysis/separation/audio_separator.py @@ -23,7 +23,6 @@ import logging import os import sys -import warnings from dataclasses import dataclass from pathlib import Path from typing import Any, cast @@ -32,7 +31,6 @@ import numpy as np from bandscope_analysis.temporal.analyzer import ( - KNOWN_LIBROSA_NUMBA_WARNING_FILTERS, MAX_ANALYSIS_DURATION_SECONDS, MAX_AUDIO_FILE_BYTES, TARGET_SR, @@ -200,24 +198,12 @@ def _load_audio(self, path: Path) -> tuple[AudioStemArray, int]: f"{file_size} bytes (max {self.config.max_file_bytes} bytes)" ) - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", category=DeprecationWarning, module=r"^audioread" - ) - warnings.filterwarnings("ignore", category=FutureWarning, module=r"^audioread") - for category, message, module in KNOWN_LIBROSA_NUMBA_WARNING_FILTERS: - warnings.filterwarnings( - "ignore", - category=category, - message=message, - module=module, - ) - y, sr = librosa.load( - fileobj, - sr=self.config.target_sample_rate, - mono=True, - duration=self.config.max_duration_seconds, - ) + y, sr = librosa.load( + fileobj, + sr=self.config.target_sample_rate, + mono=True, + duration=self.config.max_duration_seconds, + ) except ValueError: raise except Exception as error: diff --git a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py index 7fe5ae6f7..9746b48b4 100644 --- a/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py +++ b/services/analysis-engine/src/bandscope_analysis/temporal/analyzer.py @@ -4,7 +4,6 @@ import logging import os -import warnings from pathlib import Path from typing import Any @@ -20,10 +19,6 @@ TARGET_SR = 44100 MAX_AUDIO_FILE_BYTES = 100 * 1024 * 1024 # 100 MiB MAX_ANALYSIS_DURATION_SECONDS = 15 * 60 # 15 minutes -KNOWN_LIBROSA_NUMBA_WARNING_FILTERS = ( - (DeprecationWarning, r".*pkg_resources is deprecated.*", r".*librosa.*"), - (FutureWarning, r".*Numba.*", r".*numba.*"), -) # ponytail: assumes 4/4; upgrade to meter estimation or a madmom DBN if other meters matter. BEATS_PER_BAR = 4 @@ -84,28 +79,14 @@ def analyze(self, audio_path: str | Path) -> TemporalFeatures: f"(max {MAX_AUDIO_FILE_BYTES} bytes)" ) - with warnings.catch_warnings(): - warnings.filterwarnings( - "ignore", category=DeprecationWarning, module=r"^audioread" - ) - warnings.filterwarnings("ignore", category=FutureWarning, module=r"^audioread") - - # Keep the loader's known third-party churn quiet without hiding - # unrelated decoder warnings that tests and callers should see. - for category, message, module in KNOWN_LIBROSA_NUMBA_WARNING_FILTERS: - warnings.filterwarnings( - "ignore", - category=category, - message=message, - module=module, - ) - # Load audio, converting to mono and standardizing sample rate - y, sr = librosa.load( - fileobj, - sr=TARGET_SR, - mono=True, - duration=MAX_ANALYSIS_DURATION_SECONDS, - ) + # Load audio, converting to mono and standardizing sample rate. + # Warnings remain visible so test/CI policy can root-cause them. + y, sr = librosa.load( + fileobj, + sr=TARGET_SR, + mono=True, + duration=MAX_ANALYSIS_DURATION_SECONDS, + ) # Ensure it's a 1D float array for librosa if not isinstance(y, np.ndarray): diff --git a/services/analysis-engine/src/bandscope_analysis/transcription/api.py b/services/analysis-engine/src/bandscope_analysis/transcription/api.py index f2a732d31..057be226c 100644 --- a/services/analysis-engine/src/bandscope_analysis/transcription/api.py +++ b/services/analysis-engine/src/bandscope_analysis/transcription/api.py @@ -3,7 +3,6 @@ from __future__ import annotations import io -import warnings from dataclasses import dataclass import librosa @@ -42,14 +41,12 @@ def transcribe_bass_stem(stem_data: bytes) -> list[NoteEvent]: if len(stem_data) > MAX_STEM_BYTES: raise ValueError("Stem data is too large for transcription.") - with warnings.catch_warnings(): - warnings.filterwarnings("ignore", category=DeprecationWarning, module=r"^audioread") - y, sr = librosa.load( - io.BytesIO(stem_data), - sr=TARGET_SR, - mono=True, - duration=MAX_TRANSCRIPTION_DURATION_SECONDS, - ) + y, sr = librosa.load( + io.BytesIO(stem_data), + sr=TARGET_SR, + mono=True, + duration=MAX_TRANSCRIPTION_DURATION_SECONDS, + ) y_array = np.asarray(y, dtype=np.float32) if y_array.size == 0 or float(np.max(np.abs(y_array))) < MIN_SIGNAL_PEAK: diff --git a/services/analysis-engine/tests/test_deprecation_warning_policy.py b/services/analysis-engine/tests/test_deprecation_warning_policy.py new file mode 100644 index 000000000..702c2b5b5 --- /dev/null +++ b/services/analysis-engine/tests/test_deprecation_warning_policy.py @@ -0,0 +1,50 @@ +"""Tests for analysis-engine warning visibility policy.""" + +from __future__ import annotations + +import ast +import tomllib +from pathlib import Path + +_ANALYSIS_ROOT = Path(__file__).resolve().parents[1] +_AUDIO_LOADER_PATHS = ( + _ANALYSIS_ROOT / "src/bandscope_analysis/temporal/analyzer.py", + _ANALYSIS_ROOT / "src/bandscope_analysis/transcription/api.py", + _ANALYSIS_ROOT / "src/bandscope_analysis/separation/audio_separator.py", +) + + +def _is_runtime_warning_ignore(call: ast.Call) -> bool: + """Return whether one production call hides warnings at runtime.""" + if not isinstance(call.func, ast.Attribute) or call.func.attr != "filterwarnings": + return False + if not call.args or not isinstance(call.args[0], ast.Constant): + return False + return call.args[0].value == "ignore" + + +def test_pytest_fails_on_unowned_compatibility_warnings() -> None: + """Require pytest to turn unowned deprecation/future warnings into failures.""" + pyproject_path = _ANALYSIS_ROOT / "pyproject.toml" + config = tomllib.loads(pyproject_path.read_text(encoding="utf-8")) + filters = config["tool"]["pytest"]["ini_options"].get("filterwarnings", []) + + assert "error::DeprecationWarning" in filters + assert "ignore::DeprecationWarning" not in filters + assert "error::FutureWarning" in filters + assert "ignore::FutureWarning" not in filters + + +def test_audio_loaders_do_not_hide_runtime_warnings() -> None: + """Keep production audio loaders from converting warning findings into silence.""" + offenders: list[str] = [] + for path in _AUDIO_LOADER_PATHS: + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + if any( + _is_runtime_warning_ignore(node) + for node in ast.walk(tree) + if isinstance(node, ast.Call) + ): + offenders.append(str(path.relative_to(_ANALYSIS_ROOT))) + + assert offenders == []