From 62b60e3daeaa2984d5ee051ba910194d52018f04 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Sun, 12 Jul 2026 12:36:54 +0200 Subject: [PATCH] refactor(scan): extract scan_chunk from scan_library scan_chunk resolves album folders whose (artist, album) sorts after a cursor, up to a limit, returning (new_cursor, imported, skipped, done). scan_library becomes the unbounded case (limit=None), so its behavior and tests are unchanged. Idempotent: all writes are ON CONFLICT, so an overlapping/repeated cursor is safe. Enables chunked scanning in the loop. Co-Authored-By: Claude Opus 4.8 (1M context) --- worker/lyra_worker/scan.py | 100 +++++++++++++++++++++++--------- worker/tests/test_scan_chunk.py | 81 ++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 27 deletions(-) create mode 100644 worker/tests/test_scan_chunk.py diff --git a/worker/lyra_worker/scan.py b/worker/lyra_worker/scan.py index ccb0db5..ceeba75 100644 --- a/worker/lyra_worker/scan.py +++ b/worker/lyra_worker/scan.py @@ -91,16 +91,11 @@ def _record_owned(conn, target, quality_class: int, fmt: str, path: str, watched return newly -def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, dest_root: str = "/music") -> ScanResult: - """Walk `{dest_root}/{Artist}/{Album}/`, resolve each album against MusicBrainz, and record - matched albums as have + monitored + followed. Also browses each followed artist's full - discography (once per scan run) and stores it as unmonitored releases, matching web-follow - behavior. Unmatched or audio-less folders are skipped.""" - imported = 0 - skipped = 0 - browsed: set[str] = set() # artists whose discography we've populated this run +def _iter_album_entries(dest_root: str): + """Yield (artist_name, album_folder, album_path) for each candidate album directory, + in the stable nested-sorted order the scan cursor relies on.""" if not os.path.isdir(dest_root): - return ScanResult(0, 0) + return for artist_name in sorted(os.listdir(dest_root)): artist_path = os.path.join(dest_root, artist_name) if artist_name.startswith(".") or not os.path.isdir(artist_path): @@ -109,22 +104,73 @@ def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, album_path = os.path.join(artist_path, album_folder) if album_folder.startswith(".") or not os.path.isdir(album_path): continue - audio = _first_audio(album_path) - if audio is None: - continue # no audio → not an album - pr = probe.probe(audio) - album_name = _parse_album_dir(album_folder) - target = resolver.resolve(artist_name, album_name) if artist_name and album_name else None - if target is None: # fall back to the file's embedded tags - if pr.artist and pr.album and (pr.artist, pr.album) != (artist_name, album_name): - target = resolver.resolve(pr.artist, pr.album) - if target is None: - skipped += 1 - continue - watched_id = _ensure_artist(conn, target) - if watched_id is not None and target.artist_mbid not in browsed: - _populate_discography(conn, browser, watched_id, target.artist_mbid, target.artist) - browsed.add(target.artist_mbid) - if _record_owned(conn, target, pr.quality_class, pr.fmt, album_path, watched_id): - imported += 1 + yield artist_name, album_folder, album_path + + +def _cursor_key(artist_name: str, album_folder: str) -> str: + return f"{artist_name}/{album_folder}" + + +def _cursor_tuple(cursor: str) -> tuple[str, str]: + # A cursor is "/"; artist and album are single path segments (no "/"), + # so partitioning on the first "/" recovers them, and tuple order matches the walk. + artist, _, album = (cursor or "").partition("/") + return artist, album + + +def _process_album(conn, resolver, probe, browser, artist_name, album_folder, album_path, browsed) -> str: + """Identify and record one album folder. Returns 'imported' (newly recorded), + 'recorded' (matched but already present), 'skipped' (no MB match) or 'none' (no audio).""" + audio = _first_audio(album_path) + if audio is None: + return "none" # no audio → not an album + pr = probe.probe(audio) + album_name = _parse_album_dir(album_folder) + target = resolver.resolve(artist_name, album_name) if artist_name and album_name else None + if target is None: # fall back to the file's embedded tags + if pr.artist and pr.album and (pr.artist, pr.album) != (artist_name, album_name): + target = resolver.resolve(pr.artist, pr.album) + if target is None: + return "skipped" + watched_id = _ensure_artist(conn, target) + if watched_id is not None and target.artist_mbid not in browsed: + _populate_discography(conn, browser, watched_id, target.artist_mbid, target.artist) + browsed.add(target.artist_mbid) + return "imported" if _record_owned(conn, target, pr.quality_class, pr.fmt, album_path, watched_id) else "recorded" + + +def scan_chunk(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, dest_root: str = "/music", + cursor: str = "", limit: int | None = None) -> tuple[str, int, int, bool]: + """Resolve and record album folders whose (artist, album) sorts after `cursor`, up to + `limit` of them (unbounded when limit is None). Returns (new_cursor, imported, skipped, + done); `done` is True when the walk was exhausted within this chunk. All writes are + ON CONFLICT, so an overlapping or repeated cursor is idempotent.""" + after = _cursor_tuple(cursor) + imported = skipped = processed = 0 + browsed: set[str] = set() # artists whose discography we've populated this chunk + new_cursor = cursor + done = True + for artist_name, album_folder, album_path in _iter_album_entries(dest_root): + if (artist_name, album_folder) <= after: + continue # already processed by an earlier chunk + if limit is not None and processed >= limit: + done = False # stopped early; more albums remain after new_cursor + break + outcome = _process_album(conn, resolver, probe, browser, artist_name, album_folder, album_path, browsed) + if outcome == "imported": + imported += 1 + elif outcome == "skipped": + skipped += 1 + new_cursor = _cursor_key(artist_name, album_folder) + processed += 1 + return new_cursor, imported, skipped, done + + +def scan_library(conn: psycopg.Connection, resolver, probe: AudioProbe, browser, dest_root: str = "/music") -> ScanResult: + """Walk `{dest_root}/{Artist}/{Album}/` in one unbounded pass, resolving each album + against MusicBrainz and recording matched albums as have + monitored + followed (plus + each followed artist's full discography as unmonitored releases). Unmatched or audio-less + folders are skipped. This is `scan_chunk` with no limit; the worker loop uses the chunked + form so a large library doesn't block job-claim.""" + _cursor, imported, skipped, _done = scan_chunk(conn, resolver, probe, browser, dest_root, cursor="", limit=None) return ScanResult(imported, skipped) diff --git a/worker/tests/test_scan_chunk.py b/worker/tests/test_scan_chunk.py new file mode 100644 index 0000000..bfdfc3a --- /dev/null +++ b/worker/tests/test_scan_chunk.py @@ -0,0 +1,81 @@ +from lyra_worker.adapters.fakes import FakeMbBrowser +from lyra_worker.browser import ReleaseGroupInfo +from lyra_worker.scan import scan_chunk +from lyra_worker.types import MBTarget +from tests.test_scan import FakeProbe, FakeResolver, _album, _counts + + +def _tree(tmp_path): + """Three albums across two artists, in nested-sorted order: + Artist A/Alpha, Artist A/Beta, Artist B/Gamma.""" + _album(tmp_path, "Artist A", "Alpha (2001)") + _album(tmp_path, "Artist A", "Beta (2002)") + _album(tmp_path, "Artist B", "Gamma (2003)") + resolver = FakeResolver({ + ("Artist A", "Alpha"): MBTarget(artist="Artist A", album="Alpha", year=2001, rg_mbid="rg-a1", artist_mbid="ma"), + ("Artist A", "Beta"): MBTarget(artist="Artist A", album="Beta", year=2002, rg_mbid="rg-a2", artist_mbid="ma"), + ("Artist B", "Gamma"): MBTarget(artist="Artist B", album="Gamma", year=2003, rg_mbid="rg-b1", artist_mbid="mb"), + }) + browser = FakeMbBrowser(releases={ + "ma": [ReleaseGroupInfo("rg-a1", "Alpha", "Album", (), "2001"), + ReleaseGroupInfo("rg-a2", "Beta", "Album", (), "2002")], + "mb": [ReleaseGroupInfo("rg-b1", "Gamma", "Album", (), "2003")], + }) + return resolver, browser + + +def test_scan_chunk_respects_limit_and_returns_cursor(conn, tmp_path): + resolver, browser = _tree(tmp_path) + cursor, imported, skipped, done = scan_chunk( + conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), cursor="", limit=2) + assert (imported, skipped, done) == (2, 0, False) # processed 2 of 3, more remain + assert cursor == "Artist A/Beta (2002)" # last processed key + with conn.cursor() as cur: + cur.execute('SELECT count(*) FROM "LibraryItem"') + assert cur.fetchone()[0] == 2 + + +def test_scan_chunk_resumes_from_cursor(conn, tmp_path): + resolver, browser = _tree(tmp_path) + cursor, imported, skipped, done = scan_chunk( + conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), + cursor="Artist A/Beta (2002)", limit=25) + assert (imported, skipped, done) == (1, 0, True) # only Gamma remains + assert cursor == "Artist B/Gamma (2003)" + with conn.cursor() as cur: + cur.execute('SELECT album FROM "LibraryItem"') + assert cur.fetchall() == [("Gamma",)] # only Gamma imported + + +def test_scan_chunk_cursor_past_end_is_noop(conn, tmp_path): + resolver, browser = _tree(tmp_path) + cursor, imported, skipped, done = scan_chunk( + conn, resolver, FakeProbe(), browser, str(tmp_path), cursor="zzzz/zzzz", limit=10) + assert (imported, skipped, done) == (0, 0, True) + assert cursor == "zzzz/zzzz" # unchanged + assert _counts(conn)["LibraryItem"] == 0 + + +def test_scan_chunk_splits_artist_across_chunks_idempotently(conn, tmp_path): + # limit=1 forces Artist A's two albums into separate chunks, re-browsing its + # discography — which must stay idempotent (ON CONFLICT) and total the same as one scan. + resolver, browser = _tree(tmp_path) + cursor, done, total_i, total_s, guard = "", False, 0, 0, 0 + while not done and guard < 10: + cursor, i, s, done = scan_chunk( + conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), cursor=cursor, limit=1) + total_i += i + total_s += s + guard += 1 + assert (total_i, total_s) == (3, 0) + assert guard == 3 # three albums, one per chunk, then done + assert _counts(conn) == {"LibraryItem": 3, "MonitoredRelease": 3, "WatchedArtist": 2} + + +def test_scan_chunk_unbounded_matches_full_walk(conn, tmp_path): + resolver, browser = _tree(tmp_path) + cursor, imported, skipped, done = scan_chunk( + conn, resolver, FakeProbe(quality_class=2), browser, str(tmp_path), cursor="", limit=None) + assert (imported, skipped, done) == (3, 0, True) + assert cursor == "Artist B/Gamma (2003)" + assert _counts(conn) == {"LibraryItem": 3, "MonitoredRelease": 3, "WatchedArtist": 2}