fix(repair): rewire Unknown Artist Fixer deferred imports (#646)
The "Fix Unknown Artists" repair job crashed on every run with:
ImportError: cannot import name '_build_path_from_template' from
'core.repair_jobs.library_reorganize'
Commit ca5c9316 ("Rewrite Library Reorganize job to delegate to per-
album planner") moved the private path-builder + quality-string
helpers out of `core.repair_jobs.library_reorganize` and into the
import pipeline. `unknown_artist_fixer.py:163` still imported them
from the old module — its scan() defers the imports to avoid pulling
web_server's Flask boot into the test harness, so the broken target
only surfaces at runtime when the user actually runs the job. The
tool was completely unrunnable.
Re-wired the deferred imports:
core.repair_jobs.library_reorganize._build_path_from_template
-> core.imports.paths.get_file_path_from_template_raw
core.repair_jobs.library_reorganize._get_audio_quality
-> core.imports.file_ops.get_audio_quality_string
Both replacements have identical signatures + return shapes (verified
by inspecting library_reorganize's pre-refactor implementations vs
the import-pipeline equivalents):
get_file_path_from_template_raw(template: str, context: dict)
-> tuple[folder: str, filename_base: str]
get_audio_quality_string(file_path: str) -> str
No call-site changes needed beyond the import target.
2 new regression tests in `tests/test_unknown_artist_fixer.py`:
test_deferred_path_imports_resolve — runs the same import
statements scan() runs, so the NEXT refactor that moves these
helpers fails CI rather than reaching the user.
test_deferred_path_helper_shape_matches_fixer_usage — pins the
`(folder, filename_base)` 2-tuple contract the fixer's unpack
relies on. Catches return-shape drift even when the import
target stays valid.
Audited every consumer of `core.repair_jobs.library_reorganize` —
only one stale import (this file). The test suite covers the only
production caller.
5 fixer tests pass (3 existing + 2 new regression guards).
This commit is contained in:
Broque Thomas
parent
5195f68912
commit
735dd73865
3 changed files with 72 additions and 3 deletions
|
|
@ -160,8 +160,18 @@ class UnknownArtistFixerJob(RepairJob):
|
|||
# Compute expected file path
|
||||
expected_rel = None
|
||||
if reorganize_files and corrected.get('artist') and corrected.get('album'):
|
||||
from core.repair_jobs.library_reorganize import _build_path_from_template, _get_audio_quality
|
||||
quality = _get_audio_quality(resolved)
|
||||
# Issue #646: `core.repair_jobs.library_reorganize` was
|
||||
# rewritten in commit ca5c9316 ("Rewrite Library
|
||||
# Reorganize job to delegate to per-album planner") and
|
||||
# the private `_build_path_from_template` /
|
||||
# `_get_audio_quality` helpers moved out. They live in
|
||||
# the import pipeline now — same shape, different
|
||||
# module. Without this re-wire the Unknown Artist Fixer
|
||||
# crashes on first scan with `ImportError: cannot
|
||||
# import name '_build_path_from_template'`.
|
||||
from core.imports.paths import get_file_path_from_template_raw
|
||||
from core.imports.file_ops import get_audio_quality_string
|
||||
quality = get_audio_quality_string(resolved)
|
||||
tmpl_ctx = {
|
||||
'artist': corrected['artist'],
|
||||
'albumartist': corrected['artist'],
|
||||
|
|
@ -173,7 +183,7 @@ class UnknownArtistFixerJob(RepairJob):
|
|||
'quality': quality,
|
||||
'albumtype': 'Album',
|
||||
}
|
||||
folder, fname_base = _build_path_from_template(album_template, tmpl_ctx)
|
||||
folder, fname_base = get_file_path_from_template_raw(album_template, tmpl_ctx)
|
||||
file_ext = os.path.splitext(resolved)[1]
|
||||
if quality and f'[{quality}]' not in fname_base:
|
||||
fname_base = f"{fname_base} [{quality}]"
|
||||
|
|
|
|||
|
|
@ -239,3 +239,61 @@ def test_unknown_artist_fixer_supports_hydrabase_title_search(monkeypatch):
|
|||
assert result["source"] == "hydrabase_title_search"
|
||||
assert hydrabase_client.search_calls == [("Hydra Match", 5)]
|
||||
assert spotify_client.search_calls == []
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Issue #646 — deferred imports inside scan() must resolve at runtime
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_deferred_path_imports_resolve():
|
||||
"""Issue #646 regression guard. The Unknown Artist Fixer's scan()
|
||||
defers `get_file_path_from_template_raw` + `get_audio_quality_string`
|
||||
imports to keep web_server's heavy boot off the test harness — but
|
||||
that means a stale import target only surfaces at *runtime*, mid-
|
||||
scan, with an ImportError. The fixer crashed with exactly that:
|
||||
|
||||
ImportError: cannot import name '_build_path_from_template' from
|
||||
'core.repair_jobs.library_reorganize'
|
||||
|
||||
after commit ca5c9316 rewrote `library_reorganize` and moved the
|
||||
helpers into the import pipeline.
|
||||
|
||||
This test runs the same import statements scan() runs, so the next
|
||||
refactor that moves these helpers fails CI rather than reaching the
|
||||
user."""
|
||||
from core.imports.paths import get_file_path_from_template_raw # noqa: F401
|
||||
from core.imports.file_ops import get_audio_quality_string # noqa: F401
|
||||
|
||||
|
||||
def test_deferred_path_helper_shape_matches_fixer_usage():
|
||||
"""Pin the shape contract the fixer relies on: pass a template
|
||||
string + a context dict with the same keys scan() builds, expect a
|
||||
`(folder, filename_base)` tuple back. If either of those moves, the
|
||||
fixer's `folder, fname_base = ...` unpack would fail loudly here
|
||||
instead of producing a malformed expected_rel path."""
|
||||
from core.imports.paths import get_file_path_from_template_raw
|
||||
|
||||
template = "$albumartist/$albumartist - $album/$track - $title"
|
||||
tmpl_ctx = {
|
||||
"artist": "Test Artist",
|
||||
"albumartist": "Test Artist",
|
||||
"album": "Test Album",
|
||||
"title": "Test Track",
|
||||
"track_number": 1,
|
||||
"disc_number": 1,
|
||||
"year": "2026",
|
||||
"quality": "FLAC 16bit",
|
||||
"albumtype": "Album",
|
||||
}
|
||||
|
||||
result = get_file_path_from_template_raw(template, tmpl_ctx)
|
||||
|
||||
assert isinstance(result, tuple) and len(result) == 2, \
|
||||
"Must return a 2-tuple — fixer does `folder, fname_base = result`"
|
||||
folder, fname_base = result
|
||||
assert isinstance(folder, str) and isinstance(fname_base, str)
|
||||
# Folder path must include the album-artist segment from the template.
|
||||
assert "Test Artist" in folder
|
||||
# Filename base must include the title from the template.
|
||||
assert "Test Track" in fname_base
|
||||
|
|
|
|||
|
|
@ -3427,6 +3427,7 @@ const WHATS_NEW = {
|
|||
{ title: 'Fix: slskd-unreachable log spam during non-Soulseek downloads', desc: 'when slskd was configured but not actually running (or unreachable on its configured port), the `/api/downloads/status` polling loop fanned out to every download plugin including Soulseek, producing one `ERROR - Cannot connect to host ... [Name or service not known]` log line per poll for the entire duration of any download — visible spam even when the user wasn\'t using Soulseek at all. Connection failures now emit one WARNING with actionable context (start slskd, or clear the slskd_url if you don\'t use Soulseek) and demote subsequent failures to debug. The flag resets on the next successful slskd response so a later outage warns again.' },
|
||||
{ title: 'Fix: MusicBrainz "Other" release-groups now visible in discography', desc: 'MB tags music videos, one-off web releases, and broadcast singles with `primary-type=Other` — common pattern for Vocaloid producers, JP indie artists, and some Western indie acts. The release-group browse filter only requested `album|ep|single`, dropping every Other-typed group at the API layer. Combined with the inline type mapper defaulting unknown primary types to "album", this hid legitimate tracks from the artist detail page and left downloaded tracks orphaned (visible in track counts but not bound to any visible album / single card). Centralised the type-mapper into one shared helper (`core/metadata/release_type.py`) used by every provider\'s raw→Album projection, added `Other` and `Broadcast` handling that routes those release-groups into the Singles section, and added `other` to the MB API filter. For inabakumori, this surfaces 5 previously-invisible releases. 23 unit tests pin the mapper contract; existing 65 search-adapter tests still green.' },
|
||||
{ title: 'Fix: quarantined files no longer re-downloaded on auto-wishlist cycles', desc: 'when a file failed AcoustID verification and got quarantined, the next auto-wishlist run would search for the same track again, and the candidate picker\'s deterministic quality ranking kept selecting the same `(uploader, filename)` source — re-downloading and re-quarantining the exact same file every cycle. Users woke up to hundreds of duplicate `.quarantined` entries from a single bad upload. The Soulseek candidate filter now reads quarantine sidecars and drops any candidate whose `(username, filename)` matches a previously-quarantined source before the quality picker ranks it. Filesystem read on every search (sub-ms in practice). Approving or deleting a quarantine entry removes its source from the dedup set automatically — the user can give the source a second chance by explicitly approving / deleting the quarantine record.' },
|
||||
{ title: 'Fix: Unknown Artist Fixer tool crashed on every run', desc: 'the "Fix Unknown Artists" repair job crashed instantly with `ImportError: cannot import name \'_build_path_from_template\'` — totally unrunnable. Commit ca5c9316 ("Rewrite Library Reorganize job to delegate to per-album planner") moved the private path-builder + quality-string helpers out of `core.repair_jobs.library_reorganize` and into the import pipeline (`core.imports.paths` / `core.imports.file_ops`), but the Unknown Artist Fixer\'s deferred-import path was left pointing at the old module. Re-wired to the new locations. Added two regression tests that exercise the deferred imports directly — the next refactor that moves these helpers fails CI rather than reaching the user.' },
|
||||
],
|
||||
'2.5.5': [
|
||||
{ date: 'May 17, 2026 — 2.5.5 release' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue