refactor(server): extract the song routes into routers/song.py (R3) [needs device pass]

[byrongamatos]

⚠️ BEHAVIORAL — needs an on-device pass before merge (file mutations: upload / delete / metadata write-back).

Upload/delete, the catalog-metadata write-back, user-meta, overrides, gap-fill, and the per-song info payload (11 routes) → lib/routers/song.py with their exclusive helpers (atomic upload commit + song-IO lock, upload caps, gap-fill proposal builders).

Bodies verbatim except @app→@router and the seam reads: meta_db→appstate.meta_db, art_override_paths→appstate.art_override_paths, and the scan/ingest helpers that stay in server.py → new appstate seams: kick_scan, invalidate_song_caches, stat_for_cache, and scan_status() (a getter — the underlying dict is reassigned). The gap-fill MBID/ISRC regexes are reached as enrichment.X; _MULTIPART_OVERHEAD_SLACK (shared with the staying AcoustID-identify route) moves to lib/enrichment.py beside _ACOUSTID_MAX_UPLOAD_BYTES.

Route order: song_router mounts after art_router — get_song_info's catch-all /api/song/{filename:path} would otherwise shadow /api/song/{path}/art* (Starlette matches first-registered; the :path converter is greedy). The full suite's art/cover-304 + CAA-fetch tests caught this before it was fixed.

server.py: 4,478 → 3,692 (−786).

Verification

• pyflakes clean; ordered route table preserves the specific-before-catch-all invariant.
• Full pytest 2397 passed; npm run lint 0; Codex 0 findings.
• Boot smoke: /api/song/x/art 404 (not shadowed by info), /user-meta 200, /gap-fill 200, POST /songs/upload 400 (no file), 0 tracebacks.

On-device pass to run before merge

1. Upload a .sloppak (drag/drop or the upload button) → it lands in the library, scan picks it up.
2. Open the song, edit its metadata (title/artist/album) and save → confirm it writes back (reopen shows the change; the underlying feedpak is updated).
3. Set a user-meta note / difficulty / tag and a field override → confirm they persist.
4. Delete the song → confirm it's gone from the library and the file is removed.
5. Sanity: album art still shows for songs that have it (the route-ordering fix).

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added song upload and deletion with stronger validation and safer file handling.
  • Added per-song metadata, user tags, difficulty, catalog overrides, and batch editing.
  • Added metadata write-back and gap-fill preview and confirmation workflows.
  • Added song information retrieval with automatic metadata extraction and caching.

• Bug Fixes

  • Improved consistency when updating or deleting songs by synchronizing caches and scans.
  • Improved upload size validation for multipart requests.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR extracts song upload, deletion, metadata, override, gap-fill, and information routes from server.py into lib/routers/song.py, adds injected application seams, mounts the router, and centralizes multipart overhead configuration.

Changes

Song router extraction

Layer / File(s)	Summary
Router foundation and shared seams lib/appstate.py, lib/enrichment.py, lib/routers/song.py, server.py	Defines the song router, adds scan and cache injection slots, and shares the multipart overhead constant with the AcoustID size guard.
Upload and deletion flow lib/routers/song.py, server.py	Moves upload validation, atomic commit, deletion cleanup, cache invalidation, and scan coordination into the router.
User metadata and overrides lib/routers/song.py, server.py	Moves per-song metadata, catalog override, and batch-edit endpoints into the router with their validation logic.
Metadata updates and gap fill lib/routers/song.py, server.py	Moves metadata write-back, gap-fill preview and confirmation, and cached song information handling into the router.
Route mounting and extraction records server.py, CHANGELOG.md, docs/size-exemptions.md	Mounts the song router after the art router and updates extraction documentation and changelog text.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: extracting song routes from server.py into lib/routers/song.py.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/r3-router-song

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

lib/routers/song.py (1)

659-661: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

SQL-injection warnings here (and at Lines 823-824) are false positives.

The interpolated updates fragments are built only from field names drawn from fixed allowlists (("title","artist","album","year") at Line 615; {album,year,genre} at Lines 805-811), and all values go through ? placeholders. No user input reaches the query string. Consider a brief inline comment (or # noqa: S608) at both sites to suppress the recurring Ruff/OpenGrep noise.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/routers/song.py` around lines 659 - 661, Suppress the SQL-injection false
positive at the UPDATE statements in the song update flow and the corresponding
statement around the album/year/genre updates, using a brief inline
justification or the project’s supported S608 suppression. Keep the existing
fixed-field allowlists and parameterized values unchanged.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/size-exemptions.md`:
- Around line 58-59: The size-exemption history entry in docs/size-exemptions.md
has an incorrect baseline. Change the “was 14,037” value in the MetadataDB and
AudioEffectsMappingDB extraction entry to 4,478, preserving the current 3,692
count and the surrounding explanatory text.

---

Nitpick comments:
In `@lib/routers/song.py`:
- Around line 659-661: Suppress the SQL-injection false positive at the UPDATE
statements in the song update flow and the corresponding statement around the
album/year/genre updates, using a brief inline justification or the project’s
supported S608 suppression. Keep the existing fixed-field allowlists and
parameterized values unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2d564eb7-6597-42f0-8776-2c854a10d3fb

📥 Commits

Reviewing files that changed from the base of the PR and between 7258e10 and cb6ab49.

📒 Files selected for processing (6)

• CHANGELOG.md
• docs/size-exemptions.md
• lib/appstate.py
• lib/enrichment.py
• lib/routers/song.py
• server.py