Commit Graph

3 Commits

Author SHA1 Message Date
Byron Gamatos
165475d115
refactor(server): move the metadata-enrichment subsystem into lib/enrichment.py (R3) (#861)
* refactor(server): move the metadata-enrichment subsystem into lib/enrichment.py (R3)

MusicBrainz / Cover-Art-Archive / AcoustID transport, the match-scorer glue, and
the background enrichment worker (~930 lines, 61 defs) leave server.py as one
cohesive unit. Bodies are verbatim; the only changes are seam reads:

  meta_db / config_dir / sloppak_cache_dir / art_cache_dir -> appstate.<slot>
  _song_pack_art_exists / _art_override_paths (stay in server.py for the art +
    delete routes) -> appstate.<callable> (new seam slots, injected by reference)
  _env_flag -> env_compat.env_flag_compat (the existing identical helper)
  _artist_title_from_filename -> imported from metadata_db (its home)
  the User-Agent VERSION lookup: Path(__file__).parent ->
    Path(__file__).resolve().parents[1] (lib/enrichment.py -> app root)

server.py drives the worker through the module (import enrichment; the routes +
scan lifecycle call enrichment.X). Tests that faked the network on `server`
(_mb_http_get, _enrich_network_enabled, _caa_http_get, ...) now patch the same
names on `enrichment` — the module attribute is resolved at call time, so one
setattr reaches both the routes and the worker's internal callers. Acyclic:
enrichment imports appstate/appconfig/dlc_paths/metadata_db/mb_match/
acoustid_match/sloppak/loosefolder, never server.

server.py: 6,917 -> 5,988 (-929).

Verified: pyflakes clean (bar the pre-existing File/safe_join/tuning_name/ET);
route table IDENTICAL (143); full pytest 2400 passed (140 enrichment/art cases
incl the offline-safety + transport-error-pauses-pass contracts that fake the
network); test_packaging 44 passed (enrichment.py resolves under lib/); eslint 0.
Boot smoke: /api/enrichment/status + POST /kick serve from the new module.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: reset enrichment worker state between tests (CodeRabbit)

lib/enrichment.py now owns the worker, and it stays imported for the whole
session while the `server` fixtures pop-and-reimport `server` — so the cancel
Event / status dict / caches would leak across tests, and a stale `_enrich_cancel`
could short-circuit a later direct `_background_enrich()`. An autouse conftest
fixture clears that process-global state before each test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: tighten the enrichment-reset fixture (CodeRabbit)

Narrow the import guard to ImportError (not blind Exception, BLE001), and stop
clearing _caa_index_locks — it's guarded by _caa_index_locks_guard, so an
unlocked clear() would race a still-alive worker, and its per-release mutexes
carry no test state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 11:32:21 +02:00
Byron Gamatos
97a941c45d
fix(tests): join background scan/enrich workers before closing the DB (#735)
Root cause of the flaky pytest segfault (exit 139): the background scan and enrichment daemon threads (_scan_runner/_enrich_runner) use the shared MetadataDB connection, but test fixtures closed that connection in teardown without stopping them. A daemon thread mid-query on a freed SQLite conn is a native use-after-free → SIGSEGV. The app's startup kicks a scan, so almost any app-booting fixture was vulnerable. It only surfaced now because got-feedback/feedBack#728 added a push trigger, so ci/test runs on every push to main.

Fix: server.py retains the scan/enrich thread handles and adds _join_background_db_threads(); every test fixture now joins the workers before conn.close(). Verified: the full suite runs to completion (no segfault) where it previously crashed at ~25%.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-03 12:15:43 +02:00
ChrisBeWithYou
c7497c758d
v3 library: context-menu unification — Fix match, Refresh metadata, Get info, multi-version remove (#718)
* v3 library: context-menu unification — Fix match, Refresh metadata, Get info, multi-version remove (R2)

The ⋮ overflow and the native right-click menu already render from one
builder (openCardMenu), so every entry here lands in BOTH surfaces on
grid cards and tree rows alike — parity is structural, not maintained.

New entries (local library):
- Fix match… — opens the match modal in a single-song mode: no queue,
  no Skip, the search panel open and pre-filled; a pick pins the match
  exactly like the review flow (window.__fbFixMatch).
- Refresh metadata — POST /api/enrichment/refresh/{fn}: resets the
  song's match to unscanned (canonical values + candidates cleared,
  backoff zeroed) and kicks a pass. An EXPLICIT user action, so it may
  discard a manual pin — the automation never does, but the user asking
  for a re-match is the one party who owns that pin. Silent on success.
- Get info… — GET /api/chart/{fn}/fileinfo: file location + folder
  (selectable/copyable under the v3 no-select default), format, size,
  modified; for feedpaks the manifest summary (arrangements, stems,
  cover/lyrics presence, authors, and whichever identity keys are
  actually authored — mbid/isrc/genres/track/disc); plus the match
  verdict ("Matched (text, 96%)" / "Pinned by you" / "Not scanned").
  Under /api/chart because the GET /api/song/{path} catch-all would
  swallow the suffix.
- Remove from library — with the multi-version interstitial: on a
  multi-chart work, "remove the song" is ambiguous (a grouped card
  stands for several files), so a modal lists EVERY version with
  checkboxes (the card's own chart pre-ticked) and deletes exactly
  what was picked — one file or the batch. Single-chart songs keep the
  plain confirm.

Refresh + Get info are demo-mode blocked (cache mutation / path
exposure). apply_enrichment_match now zeroes `attempts` on an explicit
reset to unscanned, matching the stub upsert's identity-change rule.

5 new tests (refresh resets even a manual pin then re-matches via the
fake transport; 404s; fileinfo manifest/identity/match shapes;
traversal guard) + the demo-mode route list. Full-suite failure set
identical to the same-main baseline. tailwind.min.css regenerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* v3 context menu: review fixes — target displayed chart, harden Get-info

Three fixes from the PR review round:

- songs.js: Fix match / Refresh metadata / Get info now act on the DISPLAYED
  chart (playTarget) rather than the group representative, matching Play. On a
  grouped card where an intrinsic (tuning/arrangement) filter attached a
  display_chart, these three previously fixed/refreshed/showed-info for the
  wrong file. (__remove stays on `song`: it needs the group's work_key/
  chart_count and already pre-ticks the shown chart.)

- server.py fileinfo: 404 ("not a chart") unless the path is a sloppak or a
  loose song. The route previously stat'd ANY file under DLC_DIR, leaking its
  path/size/mtime for e.g. a notes.txt the user keeps there. `format` can no
  longer be "other".

- server.py fileinfo: the directory size sum skips symlinked entries so a link
  inside a song folder can't pull in (or leak the size of) a file outside it.
  Verified on the runtime that rglob does not descend symlinked subdirs.

+1 regression test (non-chart file -> 404).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
2026-07-02 18:36:00 +02:00