docs: add tagging, import & persistent library plan (slice 1, plan 4b)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,643 @@
|
||||
# Lyra Tagging, Import & Persistent Library Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Finish slice 1 by turning a completed download into a real library entry: persist `/music` to a host folder (so downloads survive container recreation), organize files into `Artist/Album (Year)/` with filesystem-safe names, and tag each track with canonical MusicBrainz metadata (mutagen) renamed to `## Title.ext`.
|
||||
|
||||
**Architecture:** A pure `library` module computes the album directory and safe names (fully unit-testable). The pipeline's download stage writes into that directory; the tag stage, after the completeness check, calls an injectable `Tagger` seam. Tests use a `FakeTagger`; the real `MutagenTagger` (writes tags + renames via mutagen) is covered by an opt-in test. The resolver is extended to return the per-track titles (`MBTarget.tracklist`). Everything is backward-compatible: `run_pipeline` gains `tagger=None`, and with no tagger the tag stage just skips tagging as today.
|
||||
|
||||
**Tech Stack:** Python 3.12 worker; `mutagen` (new dependency). Tests: pytest (pure layout logic offline + fake tagger) + an opt-in real-tagging test gated by `LYRA_LIVE_TESTS`. Docker Compose bind-mount for `/music`.
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Python 3.12 in the worker container; version-flexible local deps.
|
||||
- **New dependency:** `mutagen` (pure-Python, lightweight). Imported LAZILY inside the real tagger's method so importing the module / constructing the tagger stays offline. No other new deps.
|
||||
- `ranker`/`confidence`/`quality`/adapters/resolver-seam UNCHANGED except: `types.py` gains `MBTarget.tracklist`, `_musicbrainz.py` fills it. Pipeline changes are confined to the download-dest computation and the tag stage.
|
||||
- **Backward compatibility is mandatory:** `run_pipeline` gains `tagger=None` (default None); with no tagger the tag stage does not tag (as today). The album-dir change must not alter the path for the existing (no-year, safe-name) tests. All existing tests must keep passing unchanged.
|
||||
- `MBTarget` gains `tracklist: tuple[str, ...] = ()` (defaulted; existing constructions unaffected).
|
||||
- `/music` is a host bind-mount configured via a `MUSIC_DIR` env var (default `./music`), mounted into the **worker** service only (the worker is what writes/reads the library). The real `MutagenTagger` is NOT unit-tested offline; covered by an opt-in test (`LYRA_LIVE_TESTS`) and the pure layout logic. Normal runs never touch real audio.
|
||||
- Every task ends with a commit.
|
||||
|
||||
## Shared interfaces (defined by tasks below)
|
||||
|
||||
```python
|
||||
# worker/lyra_worker/library.py (Task 1)
|
||||
def safe_name(s: str) -> str: ... # filesystem-safe path component
|
||||
def album_dir(dest_root: str, target: MBTarget) -> str: ... # {dest_root}/{safe artist}/{safe album}[ (year)]
|
||||
|
||||
# worker/lyra_worker/types.py (Task 2): MBTarget gains `tracklist: tuple[str, ...] = ()`
|
||||
|
||||
# worker/lyra_worker/tagger.py (Task 2)
|
||||
class Tagger(Protocol):
|
||||
def tag_album(self, album_dir: str, target: MBTarget) -> None: ... # tag+rename the album's audio files
|
||||
|
||||
# worker/lyra_worker/pipeline.py (Tasks 1-2): dest uses album_dir(); run_pipeline(..., tagger=None); tag stage calls tagger
|
||||
|
||||
# worker/lyra_worker/_mutagen.py (Task 3)
|
||||
class MutagenTagger: # real Tagger using mutagen (lazily imported)
|
||||
def tag_album(self, album_dir, target) -> None: ...
|
||||
|
||||
# worker/lyra_worker/registry.py (Task 4): build_tagger() -> Tagger
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Persistent /music volume + organized album directory
|
||||
|
||||
**Files:**
|
||||
- Create: `worker/lyra_worker/library.py`
|
||||
- Test: `worker/tests/test_library.py`
|
||||
- Modify: `worker/lyra_worker/pipeline.py` (download dest uses `album_dir`)
|
||||
- Modify: `docker-compose.yml` (bind-mount `/music` on the worker)
|
||||
- Modify: `.env.example` (add `MUSIC_DIR`)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `MBTarget`.
|
||||
- Produces: `safe_name(s) -> str` and `album_dir(dest_root, target) -> str`; the pipeline's download `dest` becomes `album_dir(dest_root, target)`; the worker mounts `${MUSIC_DIR}:/music`.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
`worker/tests/test_library.py`:
|
||||
```python
|
||||
from lyra_worker.library import album_dir, safe_name
|
||||
from lyra_worker.types import MBTarget
|
||||
|
||||
|
||||
def test_safe_name_replaces_illegal_chars():
|
||||
assert safe_name("AC/DC") == "AC_DC"
|
||||
assert safe_name('a:b*c?"') == "a_b_c__"
|
||||
assert safe_name(" .Trailing. ") == "Trailing"
|
||||
assert safe_name("") == "Unknown"
|
||||
|
||||
|
||||
def test_album_dir_with_year():
|
||||
t = MBTarget(artist="John Mayer", album="Continuum", year=2006)
|
||||
assert album_dir("/music", t) == "/music/John Mayer/Continuum (2006)"
|
||||
|
||||
|
||||
def test_album_dir_without_year():
|
||||
t = MBTarget(artist="John Mayer", album="Continuum")
|
||||
assert album_dir("/music", t) == "/music/John Mayer/Continuum"
|
||||
|
||||
|
||||
def test_album_dir_sanitizes():
|
||||
t = MBTarget(artist="AC/DC", album="Back in Black", year=1980)
|
||||
assert album_dir("/music", t) == "/music/AC_DC/Back in Black (1980)"
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
worker/.venv/bin/python -m pytest tests/test_library.py -v
|
||||
```
|
||||
Expected: FAIL — `ModuleNotFoundError: No module named 'lyra_worker.library'`.
|
||||
|
||||
- [ ] **Step 3: Implement the library helpers**
|
||||
|
||||
`worker/lyra_worker/library.py`:
|
||||
```python
|
||||
import re
|
||||
|
||||
from lyra_worker.types import MBTarget
|
||||
|
||||
_ILLEGAL = re.compile(r'[<>:"/\\|?*\x00-\x1f]')
|
||||
|
||||
|
||||
def safe_name(s: str) -> str:
|
||||
"""A filesystem-safe path component (single segment, no separators)."""
|
||||
cleaned = _ILLEGAL.sub("_", s).strip().strip(".").strip()
|
||||
return cleaned or "Unknown"
|
||||
|
||||
|
||||
def album_dir(dest_root: str, target: MBTarget) -> str:
|
||||
"""`{dest_root}/{safe artist}/{safe album}[ (year)]`."""
|
||||
album = safe_name(target.album)
|
||||
if target.year:
|
||||
album = f"{album} ({target.year})"
|
||||
return f"{dest_root}/{safe_name(target.artist)}/{album}"
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the test to verify it passes**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
worker/.venv/bin/python -m pytest tests/test_library.py -v
|
||||
```
|
||||
Expected: PASS — all four tests green.
|
||||
|
||||
- [ ] **Step 5: Use album_dir for the download destination**
|
||||
|
||||
In `worker/lyra_worker/pipeline.py`, add the import at the top:
|
||||
```python
|
||||
from lyra_worker.library import album_dir
|
||||
```
|
||||
In the download stage, replace the dest computation. Find:
|
||||
```python
|
||||
dest = f"{dest_root}/{target.artist}/{target.album}"
|
||||
```
|
||||
Replace with:
|
||||
```python
|
||||
dest = album_dir(dest_root, target)
|
||||
```
|
||||
(Nothing else in the pipeline changes in this task.)
|
||||
|
||||
- [ ] **Step 6: Add the persistent /music bind-mount**
|
||||
|
||||
In `.env.example`, add:
|
||||
```
|
||||
# Host directory for the downloaded music library (bind-mounted into the worker at /music)
|
||||
MUSIC_DIR=./music
|
||||
```
|
||||
In `docker-compose.yml`, add a `volumes:` entry to the `worker` service (alongside `build`, `restart`, `environment`, `depends_on`):
|
||||
```yaml
|
||||
volumes:
|
||||
- ${MUSIC_DIR:-./music}:/music
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Verify the compose config and the full worker suite**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd /home/jonathan/Projects/lyra
|
||||
docker compose config -q && echo "compose OK"
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest -q
|
||||
```
|
||||
Expected: `compose OK`; the full worker suite passes (the album_dir change doesn't alter paths for the existing no-year tests, so nothing regresses), output pristine.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add worker/lyra_worker/library.py worker/tests/test_library.py worker/lyra_worker/pipeline.py docker-compose.yml .env.example
|
||||
git commit -m "feat: organized album directory and persistent /music bind-mount"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: MBTarget.tracklist + Tagger seam + pipeline tag stage
|
||||
|
||||
**Files:**
|
||||
- Modify: `worker/lyra_worker/types.py` (add `tracklist` to MBTarget)
|
||||
- Create: `worker/lyra_worker/tagger.py` (the `Tagger` Protocol)
|
||||
- Modify: `worker/lyra_worker/pipeline.py` (tag stage calls the tagger)
|
||||
- Test: `worker/tests/test_tag_stage.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `MBTarget`.
|
||||
- Produces:
|
||||
- `MBTarget.tracklist: tuple[str, ...] = ()`.
|
||||
- `tagger.Tagger` Protocol: `tag_album(album_dir, target) -> None`.
|
||||
- `run_pipeline(conn, job_id, adapters, resolver=None, tagger=None, ...)`: after the tag-stage completeness check passes, if `tagger is not None`, call `tagger.tag_album(dest, target)`. Tagger failures must NOT fail the job (log and continue) — a tagging error shouldn't discard a good download.
|
||||
|
||||
- [ ] **Step 1: Add `tracklist` to MBTarget**
|
||||
|
||||
In `worker/lyra_worker/types.py`, `MBTarget` gains a `tracklist` field (after `year`):
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class MBTarget:
|
||||
artist: str
|
||||
album: str
|
||||
track_count: int | None = None
|
||||
total_duration_s: int | None = None
|
||||
year: int | None = None
|
||||
tracklist: tuple[str, ...] = ()
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add the Tagger Protocol**
|
||||
|
||||
`worker/lyra_worker/tagger.py`:
|
||||
```python
|
||||
from typing import Protocol
|
||||
|
||||
from lyra_worker.types import MBTarget
|
||||
|
||||
|
||||
class Tagger(Protocol):
|
||||
def tag_album(self, album_dir: str, target: MBTarget) -> None:
|
||||
"""Tag the album's audio files with target's metadata and rename them."""
|
||||
...
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Write the failing tests**
|
||||
|
||||
`worker/tests/test_tag_stage.py`:
|
||||
```python
|
||||
from lyra_worker.adapters.qobuz import QobuzAdapter
|
||||
from lyra_worker.claim import claim_next
|
||||
from lyra_worker.pipeline import run_pipeline
|
||||
from tests.conftest import insert_request
|
||||
from tests.test_qobuz_adapter import FakeQobuzClient
|
||||
|
||||
|
||||
class RecordingTagger:
|
||||
def __init__(self, fail=False):
|
||||
self.calls = []
|
||||
self._fail = fail
|
||||
|
||||
def tag_album(self, album_dir, target):
|
||||
self.calls.append((album_dir, target))
|
||||
if self._fail:
|
||||
raise RuntimeError("tagging blew up")
|
||||
|
||||
|
||||
def test_tagger_called_with_album_dir_and_target(conn):
|
||||
tagger = RecordingTagger()
|
||||
job_id = insert_request(conn, artist="John Mayer", album="Continuum")
|
||||
claim_next(conn)
|
||||
run_pipeline(conn, job_id, [QobuzAdapter(FakeQobuzClient())], tagger=tagger, dest_root="/tmp/lib")
|
||||
|
||||
with conn.cursor() as cur:
|
||||
cur.execute('SELECT state FROM "Job" WHERE id = %s', (job_id,))
|
||||
assert cur.fetchone()[0] == "imported"
|
||||
assert len(tagger.calls) == 1
|
||||
album_dir, target = tagger.calls[0]
|
||||
assert album_dir == "/tmp/lib/John Mayer/Continuum"
|
||||
assert target.album == "Continuum"
|
||||
|
||||
|
||||
def test_tagger_failure_does_not_fail_the_job(conn):
|
||||
tagger = RecordingTagger(fail=True)
|
||||
job_id = insert_request(conn, artist="John Mayer", album="Continuum")
|
||||
claim_next(conn)
|
||||
run_pipeline(conn, job_id, [QobuzAdapter(FakeQobuzClient())], tagger=tagger, dest_root="/tmp/lib")
|
||||
|
||||
with conn.cursor() as cur:
|
||||
cur.execute('SELECT state FROM "Job" WHERE id = %s', (job_id,))
|
||||
assert cur.fetchone()[0] == "imported" # tagging error is logged, not fatal
|
||||
|
||||
|
||||
def test_no_tagger_still_imports(conn):
|
||||
job_id = insert_request(conn, artist="John Mayer", album="Continuum")
|
||||
claim_next(conn)
|
||||
run_pipeline(conn, job_id, [QobuzAdapter(FakeQobuzClient())], dest_root="/tmp/lib") # no tagger
|
||||
|
||||
with conn.cursor() as cur:
|
||||
cur.execute('SELECT state FROM "Job" WHERE id = %s', (job_id,))
|
||||
assert cur.fetchone()[0] == "imported"
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the tests to verify they fail**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest tests/test_tag_stage.py -v
|
||||
```
|
||||
Expected: FAIL — `run_pipeline` does not accept a `tagger` argument (TypeError).
|
||||
|
||||
- [ ] **Step 5: Add the tagger param and call it in the tag stage**
|
||||
|
||||
In `worker/lyra_worker/pipeline.py`:
|
||||
|
||||
(a) Add `tagger=None` to the signature (after `resolver`):
|
||||
```python
|
||||
def run_pipeline(
|
||||
conn: psycopg.Connection,
|
||||
job_id: str,
|
||||
adapters: Sequence[SourceAdapter],
|
||||
resolver=None,
|
||||
tagger=None,
|
||||
min_confidence: float = 0.7,
|
||||
dest_root: str = "/music",
|
||||
) -> None:
|
||||
```
|
||||
|
||||
(b) In the tag stage, after the completeness check passes (before the `# 6. import` block), add:
|
||||
```python
|
||||
if tagger is not None:
|
||||
try:
|
||||
tagger.tag_album(dest, target)
|
||||
except Exception as e: # a tagging failure must not discard a good download
|
||||
print(f"pipeline: tagging failed for job {job_id}: {e}", flush=True)
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run the tests to verify they pass**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest tests/test_tag_stage.py -v
|
||||
```
|
||||
Expected: PASS — all three tests green.
|
||||
|
||||
- [ ] **Step 7: Run the full worker suite (backward compatibility)**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest -q
|
||||
```
|
||||
Expected: PASS — every existing test still green (calls without `tagger` skip tagging), output pristine.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add worker/lyra_worker/types.py worker/lyra_worker/tagger.py worker/lyra_worker/pipeline.py worker/tests/test_tag_stage.py
|
||||
git commit -m "feat: Tagger seam and pipeline tag-stage integration"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Real MutagenTagger + resolver tracklist + dependency + opt-in test
|
||||
|
||||
**Files:**
|
||||
- Create: `worker/lyra_worker/_mutagen.py`
|
||||
- Modify: `worker/lyra_worker/_musicbrainz.py` (fill `tracklist`)
|
||||
- Create: `worker/tests/test_mutagen_live.py`
|
||||
- Modify: `worker/requirements.txt`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `mutagen` (lazily imported), `MBTarget`, `safe_name`.
|
||||
- Produces: `MusicBrainzResolver.resolve` now fills `tracklist` (tuple of track titles, in order); `MutagenTagger.tag_album(album_dir, target)` tags each audio file (sorted) with artist/albumartist/album/date/tracknumber/title (title from `target.tracklist[i]` when available, else the filename stem) via mutagen easy mode, then renames it to `NN Title.ext`.
|
||||
|
||||
- [ ] **Step 1: Add the dependency**
|
||||
|
||||
Append to `worker/requirements.txt`:
|
||||
```
|
||||
mutagen>=1.47
|
||||
```
|
||||
Install into the venv: `worker/.venv/bin/pip install -r worker/requirements.txt`
|
||||
|
||||
- [ ] **Step 2: Have the resolver fill the tracklist**
|
||||
|
||||
In `worker/lyra_worker/_musicbrainz.py`, in the release-traversal block where `tracks` is collected, also build the ordered title list and pass it to `MBTarget`. After computing `track_count`/`total_duration_s`, add:
|
||||
```python
|
||||
titles = tuple((t.get("recording") or {}).get("title", "") for t in tracks)
|
||||
```
|
||||
and add `tracklist=titles` to the returned `MBTarget(...)`. If no tracks were found, `tracklist` stays the default `()` — so also initialize `titles: tuple[str, ...] = ()` before the `if tracks:` block and set it inside, then pass `tracklist=titles`.
|
||||
|
||||
- [ ] **Step 3: Implement the real tagger**
|
||||
|
||||
The naming/numbering logic is a pure function (`plan_track_names`) so it can be unit-tested offline; only the mutagen tag-write is left to live validation.
|
||||
|
||||
`worker/lyra_worker/_mutagen.py`:
|
||||
```python
|
||||
import os
|
||||
|
||||
from lyra_worker.library import safe_name
|
||||
|
||||
_AUDIO_EXT = {".flac", ".mp3", ".m4a", ".opus", ".ogg", ".aac", ".wav"}
|
||||
|
||||
|
||||
def plan_track_names(names: list[str], tracklist: tuple[str, ...]) -> list[tuple[str, str, str, int]]:
|
||||
"""Map audio filenames -> (old_name, new_name, title, track_number).
|
||||
|
||||
Files are sorted; titles come from `tracklist` by position when available,
|
||||
else the filename stem. Pure — no filesystem or tag I/O.
|
||||
"""
|
||||
plan: list[tuple[str, str, str, int]] = []
|
||||
audio = sorted(n for n in names if os.path.splitext(n)[1].lower() in _AUDIO_EXT)
|
||||
for i, name in enumerate(audio):
|
||||
ext = os.path.splitext(name)[1].lower()
|
||||
title = tracklist[i] if i < len(tracklist) else os.path.splitext(name)[0]
|
||||
new_name = f"{i + 1:02d} {safe_name(title)}{ext}"
|
||||
plan.append((name, new_name, title, i + 1))
|
||||
return plan
|
||||
|
||||
|
||||
class MutagenTagger:
|
||||
"""Real Tagger using mutagen (lazily imported). The tag-write is not unit-tested
|
||||
offline (needs real audio); the naming logic (`plan_track_names`) is."""
|
||||
|
||||
def tag_album(self, album_dir: str, target) -> None:
|
||||
import mutagen
|
||||
|
||||
names = os.listdir(album_dir)
|
||||
for old_name, new_name, title, number in plan_track_names(names, target.tracklist):
|
||||
path = os.path.join(album_dir, old_name)
|
||||
audio = mutagen.File(path, easy=True)
|
||||
if audio is None: # unreadable/unsupported file — leave it untouched
|
||||
continue
|
||||
audio["artist"] = target.artist
|
||||
audio["albumartist"] = target.artist
|
||||
audio["album"] = target.album
|
||||
audio["title"] = title
|
||||
audio["tracknumber"] = str(number)
|
||||
if target.year:
|
||||
audio["date"] = str(target.year)
|
||||
audio.save()
|
||||
|
||||
new_path = os.path.join(album_dir, new_name)
|
||||
if os.path.abspath(new_path) != os.path.abspath(path) and not os.path.exists(new_path):
|
||||
os.rename(path, new_path)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Write the offline naming test + the opt-in real-audio test**
|
||||
|
||||
`worker/tests/test_mutagen.py` (offline — the pure naming logic):
|
||||
```python
|
||||
from lyra_worker._mutagen import plan_track_names
|
||||
|
||||
|
||||
def test_plan_uses_tracklist_titles_and_numbers():
|
||||
names = ["02-b.flac", "01-a.flac"] # unsorted on purpose
|
||||
plan = plan_track_names(names, ("Alpha", "Beta"))
|
||||
assert plan == [
|
||||
("01-a.flac", "01 Alpha.flac", "Alpha", 1),
|
||||
("02-b.flac", "02 Beta.flac", "Beta", 2),
|
||||
]
|
||||
|
||||
|
||||
def test_plan_falls_back_to_filename_stem_and_sanitizes():
|
||||
plan = plan_track_names(["10 - AC_DC.mp3"], ()) # no tracklist
|
||||
assert plan[0][1] == "01 10 - AC_DC.mp3" # stem reused, renumbered, safe
|
||||
|
||||
|
||||
def test_plan_ignores_non_audio_files():
|
||||
plan = plan_track_names(["cover.jpg", "song.opus", "notes.txt"], ())
|
||||
assert [p[0] for p in plan] == ["song.opus"]
|
||||
```
|
||||
|
||||
`worker/tests/test_mutagen_live.py` (opt-in — tags a real sample audio file the user supplies):
|
||||
```python
|
||||
import os
|
||||
import shutil
|
||||
|
||||
import pytest
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
not (os.environ.get("LYRA_LIVE_TESTS") and os.environ.get("LYRA_SAMPLE_AUDIO")),
|
||||
reason="tags a real audio file; set LYRA_LIVE_TESTS=1 + LYRA_SAMPLE_AUDIO=/path/to/a.flac to run",
|
||||
)
|
||||
|
||||
|
||||
def test_tags_and_renames_a_real_file(tmp_path):
|
||||
import mutagen
|
||||
|
||||
from lyra_worker._mutagen import MutagenTagger
|
||||
from lyra_worker.types import MBTarget
|
||||
|
||||
ext = os.path.splitext(os.environ["LYRA_SAMPLE_AUDIO"])[1]
|
||||
src = tmp_path / f"track1{ext}"
|
||||
shutil.copy(os.environ["LYRA_SAMPLE_AUDIO"], src)
|
||||
|
||||
target = MBTarget(artist="John Mayer", album="Continuum", year=2006, tracklist=("Stop This Train",))
|
||||
MutagenTagger().tag_album(str(tmp_path), target)
|
||||
|
||||
out = tmp_path / f"01 Stop This Train{ext}"
|
||||
assert out.exists()
|
||||
tagged = mutagen.File(str(out), easy=True)
|
||||
assert tagged["artist"] == ["John Mayer"]
|
||||
assert tagged["album"] == ["Continuum"]
|
||||
assert tagged["title"] == ["Stop This Train"]
|
||||
assert tagged["tracknumber"] == ["1"]
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Verify (offline) the imports, the naming tests, and that the live test is skipped**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
worker/.venv/bin/python -c "import sys; from lyra_worker._mutagen import MutagenTagger, plan_track_names; print('import ok; mutagen loaded:', 'mutagen' in sys.modules)"
|
||||
worker/.venv/bin/python -m pytest tests/test_mutagen.py tests/test_mutagen_live.py tests/test_library.py -v
|
||||
```
|
||||
Expected: prints `import ok; mutagen loaded: False` (mutagen imported lazily inside `tag_album`); `test_mutagen.py` (3 naming tests) passes; `test_mutagen_live.py` is **skipped**; library tests pass. Do NOT set the live env vars here.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add worker/lyra_worker/_mutagen.py worker/lyra_worker/_musicbrainz.py worker/tests/test_mutagen.py worker/tests/test_mutagen_live.py worker/requirements.txt
|
||||
git commit -m "feat: real mutagen tagger and MusicBrainz tracklist"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Wire the tagger into the worker + end-to-end integration
|
||||
|
||||
**Files:**
|
||||
- Modify: `worker/lyra_worker/registry.py` (add `build_tagger()`)
|
||||
- Modify: `worker/lyra_worker/main.py` (build + pass the tagger)
|
||||
- Test: `worker/tests/test_tag_pipeline.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `MutagenTagger` (Task 3), `run_pipeline` (Task 2).
|
||||
- Produces: `registry.build_tagger() -> Tagger` returning a `MutagenTagger()`; `main.run_forever()` builds it once and passes it to `run_pipeline(conn, job_id, adapters, resolver=resolver, tagger=tagger)`.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
`worker/tests/test_tag_pipeline.py`:
|
||||
```python
|
||||
import os
|
||||
|
||||
from lyra_worker.adapters.youtube import YouTubeAdapter
|
||||
from lyra_worker.claim import claim_next
|
||||
from lyra_worker.pipeline import run_pipeline
|
||||
from lyra_worker.registry import build_tagger
|
||||
from lyra_worker.types import MBTarget
|
||||
from tests.conftest import insert_request
|
||||
from tests.test_youtube_adapter import FakeYtClient
|
||||
|
||||
|
||||
def test_build_tagger_returns_a_tagger():
|
||||
t = build_tagger()
|
||||
assert hasattr(t, "tag_album")
|
||||
|
||||
|
||||
def test_download_dir_receives_files_and_tagger_runs(conn, tmp_path):
|
||||
# A fake YouTube client that actually writes a file into dest, so a real MutagenTagger-shaped
|
||||
# tagger has something to operate on. Here we use a recording tagger to assert it's invoked
|
||||
# on the album directory the download wrote to.
|
||||
written = {}
|
||||
|
||||
def fake_download(source_ref, dest, on_progress):
|
||||
os.makedirs(dest, exist_ok=True)
|
||||
open(os.path.join(dest, "track.opus"), "wb").close()
|
||||
written["dir"] = dest
|
||||
on_progress(1.0)
|
||||
return {"track_count": 1, "path": dest}
|
||||
|
||||
client = FakeYtClient(results=[{"source_ref": "yt1", "title": "Continuum", "artist": "John Mayer", "track_count": 1}])
|
||||
client.download = fake_download
|
||||
|
||||
class RecordingTagger:
|
||||
def __init__(self):
|
||||
self.seen = None
|
||||
|
||||
def tag_album(self, album_dir, target):
|
||||
self.seen = album_dir
|
||||
|
||||
tagger = RecordingTagger()
|
||||
job_id = insert_request(conn, artist="John Mayer", album="Continuum")
|
||||
claim_next(conn)
|
||||
run_pipeline(conn, job_id, [YouTubeAdapter(client)], tagger=tagger, dest_root=str(tmp_path))
|
||||
|
||||
with conn.cursor() as cur:
|
||||
cur.execute('SELECT state FROM "Job" WHERE id = %s', (job_id,))
|
||||
assert cur.fetchone()[0] == "imported"
|
||||
assert tagger.seen == written["dir"] # tagger ran on the exact directory the download wrote to
|
||||
assert os.path.isdir(tagger.seen)
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest tests/test_tag_pipeline.py -v
|
||||
```
|
||||
Expected: FAIL — `build_tagger` does not exist in `registry`.
|
||||
|
||||
- [ ] **Step 3: Add build_tagger and wire it into main**
|
||||
|
||||
In `worker/lyra_worker/registry.py`, add:
|
||||
```python
|
||||
from lyra_worker._mutagen import MutagenTagger
|
||||
from lyra_worker.tagger import Tagger
|
||||
|
||||
|
||||
def build_tagger() -> Tagger:
|
||||
"""The tagger that writes canonical metadata onto downloaded files."""
|
||||
return MutagenTagger()
|
||||
```
|
||||
|
||||
In `worker/lyra_worker/main.py`, update the import to include `build_tagger`, and in `run_forever()` build it once and pass it:
|
||||
```python
|
||||
adapters = build_adapters(get_config(conn))
|
||||
resolver = build_resolver()
|
||||
tagger = build_tagger()
|
||||
...
|
||||
run_pipeline(conn, job_id, adapters, resolver=resolver, tagger=tagger)
|
||||
```
|
||||
(Import line: `from lyra_worker.registry import build_adapters, build_resolver, build_tagger`.)
|
||||
|
||||
- [ ] **Step 4: Run the test to verify it passes**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest tests/test_tag_pipeline.py -v
|
||||
```
|
||||
Expected: PASS — both tests green.
|
||||
|
||||
- [ ] **Step 5: Run the full worker suite**
|
||||
|
||||
Run:
|
||||
```bash
|
||||
cd worker
|
||||
export DATABASE_URL=postgresql://lyra:lyra@localhost:5432/lyra
|
||||
worker/.venv/bin/python -m pytest -q
|
||||
```
|
||||
Expected: PASS — every suite green, all live tests skipped, output pristine.
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add worker/lyra_worker/registry.py worker/lyra_worker/main.py worker/tests/test_tag_pipeline.py
|
||||
git commit -m "feat: wire the mutagen tagger into the worker loop"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Notes: slice 1 complete
|
||||
|
||||
With this plan, slice 1 (the acquisition engine) is functionally complete: a request is resolved against MusicBrainz, matched and ranked across Qobuz/Soulseek/YouTube, downloaded from the best source, verified for completeness against the canonical track count, tagged with canonical metadata, organized into `Artist/Album (Year)/## Title.ext`, and persisted to a host `/music` folder. The **live validation of `MutagenTagger`** (Task 3) is a user step: run `LYRA_LIVE_TESTS=1 pytest tests/test_mutagen_live.py`, and check real downloads land tagged and organized under `${MUSIC_DIR}`. Remaining hardening (deferred, non-blocking): worker in-loop reconnect, crash-resume/orphan-reaper for jobs dying mid-pipeline, `@@unique([jobId,sourceRef])` on Candidate, settings-form error handling, a config "clear field" path, and per-track file→MB matching by duration (currently files are matched to titles by sorted order). Slice 2 (library manager: monitoring, wanted lists, auto-grab) and slice 3 (discovery) are the next major phases, each with its own spec.
|
||||
```
|
||||
Reference in New Issue
Block a user