diff --git a/lib/library_registry.py b/lib/library_registry.py index 4588d0f3..65613233 100644 --- a/lib/library_registry.py +++ b/lib/library_registry.py @@ -17,7 +17,12 @@ from typing import ClassVar import appstate -from metadata_db import MetadataDB, _tuning_group_key_sql +from metadata_db import ( + MetadataDB, _effective_tuning_cols_sql, _perspective_is_inferred_sql, + _tuning_group_key_sql, +) +import tunings as tunings_mod +from tunings import DEFAULT_PERSPECTIVE, PERSPECTIVES from routers import art as art_router import logging @@ -39,9 +44,6 @@ def _safe_art_redirect_url(url: str) -> str | None: return None -_TUNING_GROUP_KEY_SQL = _tuning_group_key_sql("songs") - - class LocalLibraryProvider: id = "local" label = "My Library" @@ -69,28 +71,43 @@ def query_albums(self, **kwargs) -> tuple[list[dict], int]: def query_stats(self, **kwargs) -> dict: return self._db.query_stats(**kwargs) - def tuning_names(self) -> dict: + def tuning_names(self, instrument: str = DEFAULT_PERSPECTIVE) -> dict: # Group custom tunings on their raw offsets so distinct ones stay # distinct (tuning_name collapses them all to "Custom Tuning"); named # tunings keep grouping by name (stable across the rescan boundary, no # offsets/name split). `key` is the value the client sends back as the # filter selector — equal to the name for named tunings, the offsets # string for customs; offsets also feed the client's custom-pill label. + # + # `instrument=bass` swaps every column for its effective bass-facing + # expression (bass arrangement's tuning, guitar fallback) — the SAME + # expressions _build_intrinsic_where filters on, so a facet entry + # always selects exactly the songs it counted. + name_sql, offsets_sql, sort_sql = _effective_tuning_cols_sql("songs", instrument) + gkey_sql = _tuning_group_key_sql("songs", instrument) + # How many of a row's songs are showing an INFERRED tuning — i.e. have + # no bass chart of their own and are falling back to the guitar-derived + # one. Reported per entry so the UI can be honest about it instead of + # presenting a borrowed tuning as a measured one. Always 0 for guitar. + inferred_sql = f"SUM({_perspective_is_inferred_sql('songs', instrument)})" with self._db._lock: rows = self._db.conn.execute( - f"SELECT tuning_name, {_TUNING_GROUP_KEY_SQL} AS gkey, " - "MIN(tuning_sort_key), COUNT(*), MIN(tuning_offsets) " - "FROM songs WHERE title != '' AND COALESCE(tuning_name, '') != '' " + f"SELECT {name_sql}, {gkey_sql} AS gkey, " + f"MIN({sort_sql}), COUNT(*), MIN({offsets_sql}), {inferred_sql} " + f"FROM songs WHERE title != '' AND COALESCE({name_sql}, '') != '' " "GROUP BY gkey COLLATE NOCASE " - "ORDER BY ABS(COALESCE(MIN(tuning_sort_key), 0)), " - "COALESCE(MIN(tuning_sort_key), 0) ASC, " - "tuning_name COLLATE NOCASE" + f"ORDER BY ABS(COALESCE(MIN({sort_sql}), 0)), " + f"COALESCE(MIN({sort_sql}), 0) ASC, " + f"{name_sql} COLLATE NOCASE" ).fetchall() return { + "instrument": instrument, "tunings": [ {"name": name, "key": gkey, "offsets": offs or "", - "sort_key": int(sk or 0), "count": count} - for name, gkey, sk, count, offs in rows + "sort_key": int(sk or 0), "count": count, + # Portion of `count` borrowed from the guitar chart. + "inferred_count": int(inferred or 0)} + for name, gkey, sk, count, offs, inferred in rows ], } @@ -330,9 +347,16 @@ def update(self, collection: dict) -> None: # have been hand-edited; never let a bad value reach a query. self._rules = _sanitize_collection_rules(collection.get("rules") or {}) - def _filter_kwargs(self) -> dict: - return _library_filter_args(**{k: v for k, v in self._rules.items() + def _filter_kwargs(self, instrument: str = "", playable_from_pitch=None) -> dict: + # `instrument` is the CALLER's play perspective (rides every request), + # never part of the saved rules — a collection saved by a guitarist + # must still read in bass tunings for a bass player, and vice versa. + args = _library_filter_args(**{k: v for k, v in self._rules.items() if k in _LIBRARY_FILTER_PARAM_KEYS}) + args["instrument"] = _normalize_instrument(instrument) + # The caller's CURRENT tuning is likewise per-request, never a saved rule. + args["playable_from_pitch"] = playable_from_pitch + return args def _sort(self, fallback: str) -> str: # A collection may pin its own sort (e.g. "recently added"); query_page @@ -340,28 +364,31 @@ def _sort(self, fallback: str) -> str: return self._rules.get("sort") or fallback def query_page(self, *, page=0, size=24, sort="artist", direction="asc", - naming_mode="legacy", **_ignore): + naming_mode="legacy", instrument="", playable_from_pitch=None, **_ignore): return self._local._db.query_page( page=page, size=size, sort=self._sort(sort), direction=direction, - naming_mode=naming_mode, **self._filter_kwargs()) + naming_mode=naming_mode, **self._filter_kwargs(instrument, playable_from_pitch)) - def query_artists(self, *, letter="", page=0, size=50, naming_mode="legacy", **_ignore): + def query_artists(self, *, letter="", page=0, size=50, naming_mode="legacy", + instrument="", playable_from_pitch=None, **_ignore): return self._local._db.query_artists( letter=letter, page=page, size=size, naming_mode=naming_mode, - **self._filter_kwargs()) + **self._filter_kwargs(instrument, playable_from_pitch)) - def query_albums(self, *, page=0, size=120, naming_mode="legacy", **_ignore): + def query_albums(self, *, page=0, size=120, naming_mode="legacy", + instrument="", playable_from_pitch=None, **_ignore): return self._local._db.query_albums( - page=page, size=size, naming_mode=naming_mode, **self._filter_kwargs()) + page=page, size=size, naming_mode=naming_mode, + **self._filter_kwargs(instrument, playable_from_pitch)) def query_stats(self, *, sort="artist", want_sort_letters=False, - naming_mode="legacy", **_ignore): + naming_mode="legacy", instrument="", playable_from_pitch=None, **_ignore): return self._local._db.query_stats( sort=self._sort(sort), want_sort_letters=want_sort_letters, - naming_mode=naming_mode, **self._filter_kwargs()) + naming_mode=naming_mode, **self._filter_kwargs(instrument, playable_from_pitch)) - def tuning_names(self): - return self._local.tuning_names() + def tuning_names(self, instrument: str = "guitar"): + return self._local.tuning_names(instrument=_normalize_instrument(instrument)) async def get_art(self, song_id: str): return await self._local.get_art(song_id) @@ -390,7 +417,10 @@ def _library_filter_args(q: str = "", favorites: int = 0, format: str = "", artist: str = "", album: str = "", arrangements_has: str = "", arrangements_lacks: str = "", stems_has: str = "", stems_lacks: str = "", - has_lyrics: str = "", tunings: str = "") -> dict: + has_lyrics: str = "", tunings: str = "", + instrument: str = "", tuning_match: str = "", + playable_offsets: str = "", playable_instrument: str = "", + playable_string_count: str = "") -> dict: fmt = format if format in ("archive", "sloppak", "loose") else "" return { "q": q, @@ -404,9 +434,58 @@ def _library_filter_args(q: str = "", favorites: int = 0, format: str = "", "stems_lacks": _split_csv(stems_lacks), "has_lyrics": _parse_has_lyrics(has_lyrics), "tunings": _split_csv(tunings), + # Which perspective the tuning facet/filter/sort speaks for (the + # caller's play role, NOT a saved rule — see _sanitize_collection_rules). + "instrument": _normalize_instrument(instrument), + # "Playable without retuning" mode: the caller's CURRENT tuning, + # resolved to the one number the comparison needs. None = exact-match + # mode (the default), so the tuning pills behave exactly as before. + "playable_from_pitch": ( + _playable_from_pitch(playable_offsets, playable_instrument, + playable_string_count) + if tuning_match == "playable" else None), } +def _playable_from_pitch(offsets_csv: str, instrument: str, string_count: str): + """Lowest open-string MIDI pitch of the CALLER's current tuning. + + The client sends its live working tuning (offsets + instrument + string + count) rather than a precomputed pitch, so the pitch tables stay in one + place (lib/tunings.py) instead of being duplicated in JS. + + Returns None for anything unusable — the caller then applies NO playable + filter at all. That is the neutral state, not a claim: a malformed tuning + must not silently assert that everything is playable OR that nothing is. + """ + try: + offsets = [int(x) for x in _split_csv(offsets_csv)] + except (TypeError, ValueError): + return None + if not offsets: + return None + inst = "bass" if instrument == "bass" else "guitar" + try: + sc = int(string_count) + except (TypeError, ValueError): + sc = len(offsets) + key = tunings_mod.instrument_key(inst, sc) + if key not in tunings_mod.STANDARD_OPEN_MIDIS or len(offsets) != sc: + return None + midis = tunings_mod.tuning_midis_from_offsets(key, offsets) + return min(midis) if midis else None + + +def _normalize_instrument(raw: str) -> str: + """Resolve a tuning PERSPECTIVE id (guitar-lead | guitar-rhythm | bass). + + Tolerates the legacy two-valued vocabulary ("guitar" -> guitar-lead) and + falls back to the default for anything unknown — an unrecognised value + must never silently change filter semantics.""" + return raw if raw in PERSPECTIVES else ( + DEFAULT_PERSPECTIVE if raw != "bass" else "bass") + + def _sync_collection_provider(collection: dict) -> None: """Register (or replace) the provider for one collection.""" appstate.library_providers.register( diff --git a/lib/loosefolder.py b/lib/loosefolder.py index 0c456674..35fe6ba7 100644 --- a/lib/loosefolder.py +++ b/lib/loosefolder.py @@ -225,13 +225,18 @@ def _detect_arrangements(path: Path) -> tuple[list[dict], dict]: Returns (arrangements_list, shared_meta). shared_meta contains title/artist/album/year/duration/tuning_offsets sourced from the highest-priority arrangement (lead > combo > rhythm > - bass) — picking the guitar tuning when both bass and lead are present. + bass) — picking the guitar tuning when both bass and lead are present — + plus `bass_tuning_offsets` from the first bass arrangement (None when the + folder has none), so the index can carry both tunings. """ arrangements = [] # Track which arrangement priority sourced shared_meta so a later, # higher-priority arrangement (lead < bass in sort order) overrides. shared_meta = {} shared_priority = None + # First tuning seen per arrangement ROLE, kept alongside the guitar-first + # song tuning so the library can answer for the part a player plays. + role_tunings: dict[str, list[int] | None] = {"bass": None, "rhythm": None} for xml in sorted(_iter_local_xmls(path)): # Trust the XML root over the filename — a custom named @@ -269,6 +274,10 @@ def _detect_arrangements(path: Path) -> tuple[list[dict], dict]: "duration", "tuning_offsets")} shared_priority = priority + if (arr_type in role_tunings and role_tunings[arr_type] is None + and meta.get("tuning_offsets")): + role_tunings[arr_type] = list(meta["tuning_offsets"]) + arrangements.append({ "type": arr_type, "name": arr_name, @@ -281,6 +290,8 @@ def _detect_arrangements(path: Path) -> tuple[list[dict], dict]: a["index"] = i del a["priority"] + for role, offs in role_tunings.items(): + shared_meta[f"{role}_tuning_offsets"] = offs return arrangements, shared_meta @@ -412,6 +423,14 @@ def extract_meta(path: Path, dlc_root: Path | None = None) -> dict: xml_meta.get("duration", 0)) tuning_offsets = _coerce_tuning_offsets(manifest.get("tuning_offsets"), xml_meta.get("tuning_offsets")) + # Per-role tunings: XML-derived only. A manifest `tuning_offsets` overrides + # the SONG tuning (above) but says nothing about WHICH chart it describes, + # so it must never be mistaken for a specific part's tuning. + role_tunings = {} + for role in ("bass", "rhythm"): + offs = xml_meta.get(f"{role}_tuning_offsets") + role_tunings[f"{role}_tuning_offsets"] = ( + offs if isinstance(offs, list) and offs else None) manifest_arr = _validate_manifest_arrangements(manifest.get("arrangements")) if manifest_arr is not None: @@ -427,6 +446,7 @@ def extract_meta(path: Path, dlc_root: Path | None = None) -> dict: "year": year, "duration": duration, "tuning_offsets": tuning_offsets, + **role_tunings, # None = no arrangement in that role "arrangements": arrangements, "audio_path": str(audio) if audio else None, "art_path": str(art) if art else None, diff --git a/lib/metadata_db.py b/lib/metadata_db.py index ccdbbb9e..6bb021a8 100644 --- a/lib/metadata_db.py +++ b/lib/metadata_db.py @@ -25,6 +25,8 @@ from pathlib import Path from song import compute_smart_names +from tunings import DEFAULT_PERSPECTIVE, ROLE_PERSPECTIVES +from tunings import perspective as _perspective log = logging.getLogger("feedBack.server") @@ -34,13 +36,91 @@ # raw offsets so distinct customs stay distinct, while named tunings keep # grouping by name (stable across the offsets-column migration). Used by both # the tuning-names listing and the filter WHERE so the contract matches. -def _tuning_group_key_sql(alias: str) -> str: - """The tuning grouping key (name for named tunings, raw offsets for - customs) against an explicit table alias — the grouped filter law (§7.1) - evaluates chart-intrinsic predicates inside a member subquery, where bare - column names would resolve against the wrong scope.""" - return (f"CASE WHEN {alias}.tuning_name = 'Custom Tuning' AND COALESCE({alias}.tuning_offsets, '') != '' " - f"THEN {alias}.tuning_offsets ELSE {alias}.tuning_name END") +# +# A non-default PERSPECTIVE (guitar-rhythm / bass) swaps every tuning column +# for its EFFECTIVE expression: that role's indexed tuning when the song has +# such an arrangement, falling back to the guitar-derived song tuning +# otherwise — so a song with no rhythm/bass chart (or a row that predates the +# columns, NULL there) still groups/filters/sorts instead of disappearing. +# guitar-lead reads the original unprefixed columns, so it is byte-identical +# to the historical behaviour. +def _effective_tuning_cols_sql(alias: str, perspective: str = DEFAULT_PERSPECTIVE) -> tuple[str, str, str]: + """(name_sql, offsets_sql, sort_key_sql) for the given perspective.""" + persp = _perspective(perspective) + if not persp.column_prefix: + return (f"{alias}.tuning_name", f"{alias}.tuning_offsets", f"{alias}.tuning_sort_key") + has_own = f"COALESCE({alias}.{persp.column('name')}, '') != ''" + return ( + f"COALESCE(NULLIF({alias}.{persp.column('name')}, ''), {alias}.tuning_name)", + f"CASE WHEN {has_own} THEN {alias}.{persp.column('offsets')} ELSE {alias}.tuning_offsets END", + f"CASE WHEN {has_own} THEN {alias}.{persp.column('sort_key')} ELSE {alias}.tuning_sort_key END", + ) + + +def _effective_low_pitch_sql(alias: str, perspective: str = DEFAULT_PERSPECTIVE) -> str: + """Lowest open-string MIDI pitch under this perspective, with the same + fallback as the tuning columns — the "playable without retuning" + comparison reads it (see tunings.chart_is_playable_in).""" + persp = _perspective(perspective) + if not persp.column_prefix: + return f"{alias}.tuning_low_pitch" + has_own = f"COALESCE({alias}.{persp.column('name')}, '') != ''" + return (f"CASE WHEN {has_own} THEN {alias}.{persp.column('low_pitch')} " + f"ELSE {alias}.tuning_low_pitch END") + + +def _perspective_is_inferred_sql(alias: str, perspective: str) -> str: + """1 when this row is BORROWING the guitar-derived song tuning because it + has no chart in the perspective's role. Always 0 for guitar-lead, which is + never a fallback.""" + persp = _perspective(perspective) + if not persp.column_prefix: + return "0" + return f"(CASE WHEN COALESCE({alias}.{persp.column('name')}, '') = '' THEN 1 ELSE 0 END)" + + +# ── The custom-tuning group key ────────────────────────────────────────────── +# +# Named tunings group by NAME, which is already serialization-agnostic. Custom +# tunings group on a raw offsets STRING, which is not: the same physical bass +# tuning stored as "-2 0 0 0" and "-2 0 0 0 0 0" would fragment into two facet +# rows with split counts. +# +# For BASS we therefore group customs on `bass_tuning_key` — the tuning's +# absolute open-string PITCHES, computed once at scan time +# (tunings.bass_tuning_key) after the padded tail is truncated away. Pitch is +# the identity that matters musically and it is serialization-independent, so +# one physical tuning is one entry however it was authored. Guitar keeps the +# offsets string (unchanged; six-element guitar arrays are not padded). +# +# The key is built HERE, once, and read by the facet listing, the filter WHERE +# and the grouped member-match alike — a facet row that selected a different +# set than it counted is exactly the bug this shared expression prevents. +def _tuning_group_key_sql(alias: str, perspective: str = DEFAULT_PERSPECTIVE) -> str: + """The tuning grouping key (name for named tunings, canonical pitches or + raw offsets for customs) against an explicit table alias — the grouped + filter law (§7.1) evaluates chart-intrinsic predicates inside a member + subquery, where bare column names would resolve against the wrong scope.""" + persp = _perspective(perspective) + name_sql, offsets_sql, _ = _effective_tuning_cols_sql(alias, perspective) + if persp.column_prefix: + # Fall back to the offsets string when the canonical key is absent + # (a fallback row borrowing the guitar tuning, or a row scanned before + # the key column existed) so a custom never groups under an empty key. + offsets_sql = (f"COALESCE(NULLIF({alias}.{persp.column('key')}, ''), " + f"{offsets_sql})") + return (f"CASE WHEN {name_sql} = 'Custom Tuning' AND COALESCE({offsets_sql}, '') != '' " + f"THEN {offsets_sql} ELSE {name_sql} END") + + +def _put_perspective_value(meta: dict, col: str): + """Value to store for one per-perspective column on a freshly-scanned row.""" + if col.endswith("_low_pitch"): + val = meta.get(col) + return int(val) if isinstance(val, int) else None + if col.endswith("_sort_key"): + return int(meta.get(col, 0) or 0) + return meta.get(col, "") or "" # ── SQLite metadata cache ───────────────────────────────────────────────────── @@ -381,7 +461,18 @@ def __init__(self, config_dir: Path): tuning_offsets TEXT DEFAULT '', genre TEXT DEFAULT '', track_number INTEGER, - disc INTEGER + disc INTEGER, + bass_tuning_name TEXT, + bass_tuning_sort_key INTEGER, + bass_tuning_offsets TEXT, + bass_tuning_key TEXT, + bass_tuning_low_pitch INTEGER, + rhythm_tuning_name TEXT, + rhythm_tuning_sort_key INTEGER, + rhythm_tuning_offsets TEXT, + rhythm_tuning_key TEXT, + rhythm_tuning_low_pitch INTEGER, + tuning_low_pitch INTEGER ) """) # Idempotent migrations for installs that predate each column. @@ -408,6 +499,32 @@ def __init__(self, config_dir: Path): # falls back to title order. Cache; repopulated on rescan. "ALTER TABLE songs ADD COLUMN track_number INTEGER", "ALTER TABLE songs ADD COLUMN disc INTEGER", + # Bass-arrangement tuning (the KwasimodoZAZA report): the song-level + # tuning columns above are guitar-first, so the library filter lied + # to bass players when the bass chart is tuned differently. Caches; + # repopulated on rescan. NULL (no literal default) is deliberate — + # it marks a pre-migration row the scanner must re-extract, while + # '' means "extracted, song has no bass arrangement" (see scan.py). + "ALTER TABLE songs ADD COLUMN bass_tuning_name TEXT", + "ALTER TABLE songs ADD COLUMN bass_tuning_sort_key INTEGER", + "ALTER TABLE songs ADD COLUMN bass_tuning_offsets TEXT", + # Canonical grouping key: the bass tuning's absolute open-string + # pitches. Keyed on PITCH, not the serialization-dependent offsets + # string, so one physical tuning is one facet entry however it was + # stored. See tunings.bass_tuning_key. + "ALTER TABLE songs ADD COLUMN bass_tuning_key TEXT", + # Lowest open-string MIDI pitch per perspective — the "playable + # without retuning" comparison (tunings.chart_is_playable_in). + "ALTER TABLE songs ADD COLUMN bass_tuning_low_pitch INTEGER", + "ALTER TABLE songs ADD COLUMN tuning_low_pitch INTEGER", + # The RHYTHM chart's own tuning: lead and rhythm arrangements can + # be tuned differently, which is the same bug a bassist hit, + # inside guitar. Same NULL-vs-'' contract as the bass family. + "ALTER TABLE songs ADD COLUMN rhythm_tuning_name TEXT", + "ALTER TABLE songs ADD COLUMN rhythm_tuning_sort_key INTEGER", + "ALTER TABLE songs ADD COLUMN rhythm_tuning_offsets TEXT", + "ALTER TABLE songs ADD COLUMN rhythm_tuning_key TEXT", + "ALTER TABLE songs ADD COLUMN rhythm_tuning_low_pitch INTEGER", ): try: self.conn.execute(ddl) @@ -2810,16 +2927,39 @@ def continue_session(self) -> dict | None: def favorite_set(self) -> set[str]: return {r[0] for r in self.conn.execute("SELECT filename FROM favorites").fetchall()} + # Every per-perspective column, in one place, so the SELECT, the INSERT and + # the scanner's "was this ever extracted?" check can never drift apart. + # NULL is meaningful on `name`/`key`/`low_pitch`: it marks a row written + # before the column existed, which the scanner re-extracts (see + # scan._has_unextracted_columns). '' / 0 means "extracted, no such chart". + _PERSPECTIVE_COLS = tuple( + p.column(suffix) + for p in ROLE_PERSPECTIVES + for suffix in ("name", "sort_key", "offsets", "key", "low_pitch") + ) + ("tuning_low_pitch",) + # Columns whose NULL means "never extracted" rather than "no such chart". + # + # low_pitch is deliberately NOT a marker: a song with no chart in that role + # legitimately has NULL there (nothing to compute a pitch from), so keying + # re-extraction on it would re-scan those rows on every single pass and + # never converge. `name` and `key` carry the signal instead — they are '' + # when extracted-but-absent, NULL only when the column predates the row. + _EXTRACTION_MARKER_COLS = tuple( + p.column(suffix) for p in ROLE_PERSPECTIVES for suffix in ("name", "key") + ) + def get(self, filename: str, mtime: float, size: int) -> dict | None: cache_key = str(filename) + pcols = ", ".join(self._PERSPECTIVE_COLS) with self._lock: row = self.conn.execute( "SELECT mtime, size, title, artist, album, year, duration, tuning, arrangements, has_lyrics, " - "format, stem_count, stem_ids, tuning_name, tuning_sort_key, tuning_offsets " + "format, stem_count, stem_ids, tuning_name, tuning_sort_key, tuning_offsets, " + f"{pcols} " "FROM songs WHERE filename = ?", (cache_key,) ).fetchone() if row and row[0] == mtime and row[1] == size and row[2]: - return { + out = { "title": row[2], "artist": row[3], "album": row[4], "year": row[5], "duration": row[6], "tuning": row[7], "arrangements": json.loads(row[8]) if row[8] else [], @@ -2831,6 +2971,15 @@ def get(self, filename: str, mtime: float, size: int) -> dict | None: "tuning_sort_key": int(row[14] or 0), "tuning_offsets": row[15] or "", } + for i, col in enumerate(self._PERSPECTIVE_COLS, start=16): + val = row[i] + if col in self._EXTRACTION_MARKER_COLS: + out[col] = val # NULL preserved — drives re-extraction + elif col.endswith("_sort_key"): + out[col] = int(val or 0) + else: + out[col] = val or "" + return out return None def put(self, filename: str, mtime: float, size: int, meta: dict): @@ -2838,8 +2987,9 @@ def put(self, filename: str, mtime: float, size: int, meta: dict): self.conn.execute( "INSERT OR REPLACE INTO songs " "(filename, mtime, size, title, artist, album, year, duration, tuning, arrangements, " - "has_lyrics, format, stem_count, stem_ids, tuning_name, tuning_sort_key, tuning_offsets, genre, track_number, disc) " - "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + "has_lyrics, format, stem_count, stem_ids, tuning_name, tuning_sort_key, tuning_offsets, genre, track_number, disc, " + + ", ".join(self._PERSPECTIVE_COLS) + ") " + "VALUES (" + ", ".join(["?"] * (20 + len(self._PERSPECTIVE_COLS))) + ")", (filename, mtime, size, meta.get("title", ""), meta.get("artist", ""), meta.get("album", ""), meta.get("year", ""), meta.get("duration", 0), meta.get("tuning", ""), json.dumps(meta.get("arrangements", [])), @@ -2852,7 +3002,14 @@ def put(self, filename: str, mtime: float, size: int, meta: dict): meta.get("tuning_offsets", "") or "", meta.get("genre", "") or "", meta.get("track_number"), - meta.get("disc")), + meta.get("disc"), + # A put() row is by definition freshly extracted, so the + # marker columns must never be written NULL — that state is + # reserved for rows predating the column, which re-extract. + # low_pitch is the exception: NULL there means "this tuning + # has no computable pitch" (unusable offsets), and the + # playable filter treats unknown as not-playable. + *[_put_perspective_value(meta, col) for col in self._PERSPECTIVE_COLS]), ) self.conn.commit() # A song's identity may have changed → the grouping read-model is stale. @@ -3332,6 +3489,8 @@ def _build_where(self, q: str = "", favorites_only: bool = False, match_states: list[str] | None = None, genre: list[str] | None = None, naming_mode: str = "legacy", + instrument: str = DEFAULT_PERSPECTIVE, + playable_from_pitch: int | None = None, include_intrinsic: bool = True) -> tuple[str, list]: """Shared WHERE-clause builder for query_page / query_artists / query_stats. Returns (where_sql, params). Leading 'WHERE' is @@ -3438,7 +3597,8 @@ def _build_where(self, q: str = "", favorites_only: bool = False, "songs", format_filter=format_filter, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode) + has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode, + instrument=instrument, playable_from_pitch=playable_from_pitch) where += ifrag params += iparams return where, params @@ -3450,7 +3610,9 @@ def _build_intrinsic_where(self, alias: str, format_filter: str = "", stems_lacks: list[str] | None = None, has_lyrics: int | None = None, tunings: list[str] | None = None, - naming_mode: str = "legacy") -> tuple[str, list]: + naming_mode: str = "legacy", + instrument: str = DEFAULT_PERSPECTIVE, + playable_from_pitch: int | None = None) -> tuple[str, list]: """CHART-INTRINSIC predicates (format / arrangements / stems / lyrics / tuning) as ' AND …' fragments against an explicit table alias. Flat queries apply them to `songs` directly; grouped queries evaluate them @@ -3593,10 +3755,32 @@ def _build_intrinsic_where(self, alias: str, format_filter: str = "", placeholders = ",".join(["?"] * len(tn)) # Match the same grouping key tuning_names() returns so a single # "Custom Tuning" pill selects exactly its offset set while named - # tunings still match by name. - where += (f" AND {_tuning_group_key_sql(alias)} " + # tunings still match by name. `instrument` swaps in the + # effective bass tuning key (guitar fallback) — the facet and + # this WHERE must use the same expression or they disagree. + where += (f" AND {_tuning_group_key_sql(alias, instrument)} " f"COLLATE NOCASE IN ({placeholders})") params += tn + if playable_from_pitch is not None: + # "Playable without retuning" — the mode the tester actually wants + # ("don't make me retune"), offered ALONGSIDE exact match, not + # instead of it. A chart needs no retune when its lowest required + # pitch is reachable, and every pitch above your lowest open string + # is reachable by fretting, so the comparison is: + # + # your lowest open pitch <= the chart's lowest open pitch + # + # That is why a 5-string bass (low B) covers every 4-string + # standard AND every drop-D chart untouched. + # + # CONSERVATIVE BY CONSTRUCTION: a chart whose low pitch we could + # not compute (NULL) is EXCLUDED rather than assumed playable — + # wrongly claiming playability costs a mid-practice retune, which + # is the failure this whole feature exists to prevent. See + # tunings.chart_is_playable_in for the full reasoning + limits. + low_sql = _effective_low_pitch_sql(alias, instrument) + where += f" AND {low_sql} IS NOT NULL AND {low_sql} >= ?" + params.append(int(playable_from_pitch)) return where, params # Under group=1, chart-intrinsic filters match if ANY member of the work @@ -3864,7 +4048,9 @@ def query_page(self, q: str = "", page: int = 0, size: int = 24, genre: list[str] | None = None, after: str | None = None, group: bool = False, - naming_mode: str = "legacy") -> tuple[list[dict], int]: + naming_mode: str = "legacy", + instrument: str = DEFAULT_PERSPECTIVE, + playable_from_pitch: int | None = None) -> tuple[list[dict], int]: """Server-side paginated search. Returns (songs, total_count). `after` is an opaque keyset cursor (the last row of the previous page). @@ -3893,7 +4079,9 @@ def query_page(self, q: str = "", page: int = 0, size: int = 24, has_lyrics=has_lyrics, tunings=tunings, mastery=mastery, tags_has=tags_has, user_difficulty_in=user_difficulty_in, match_states=match_states, genre=genre, - naming_mode=naming_mode, include_intrinsic=not group, + naming_mode=naming_mode, instrument=instrument, + playable_from_pitch=playable_from_pitch, + include_intrinsic=not group, ) ifrag, iparams = "", [] if group: @@ -3902,12 +4090,14 @@ def query_page(self, q: str = "", page: int = 0, size: int = 24, "m", format_filter=format_filter, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode) + has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode, + instrument=instrument, playable_from_pitch=playable_from_pitch) mfrag, mparams = self._grouped_member_match(ifrag, iparams) where += mfrag params += mparams where += self._GROUP_REP_PREDICATE + _eff_tuning_name, _, _eff_tuning_sort = _effective_tuning_cols_sql("songs", instrument) sort_map = { # Artist sorts order WITHIN an artist by title (the tree view's # artist -> album -> title feel) instead of raw filename — the @@ -3941,11 +4131,15 @@ def query_page(self, q: str = "", page: int = 0, size: int = 24, # behind, and a NULL `tuning_name` in `(tuning_name = '')` # evaluates to NULL itself (which sorts ahead of 0 in # ASC), defeating the push-to-bottom intent. + # + # Under `instrument=bass` the effective expressions swap in + # the bass arrangement's tuning (guitar fallback) so a bass + # player's tuning sort orders by the tuning they'd play. "tuning": ( - "(COALESCE(tuning_name, '') = '') ASC, " - "ABS(COALESCE(tuning_sort_key, 0)), " - "COALESCE(tuning_sort_key, 0) ASC, " - "COALESCE(tuning_name, '') COLLATE NOCASE" + f"(COALESCE({_eff_tuning_name}, '') = '') ASC, " + f"ABS(COALESCE({_eff_tuning_sort}, 0)), " + f"COALESCE({_eff_tuning_sort}, 0) ASC, " + f"COALESCE({_eff_tuning_name}, '') COLLATE NOCASE" ), # Year sort (feedBack#128). Empty-year rows pushed to the # bottom for both directions; otherwise CAST so '2010' > @@ -4038,7 +4232,9 @@ def query_page(self, q: str = "", page: int = 0, size: int = 24, cols = ("SELECT filename, title, artist, album, year, duration, tuning, " "arrangements, has_lyrics, mtime, format, stem_count, stem_ids, " - "tuning_name, tuning_offsets FROM songs ") + "tuning_name, tuning_offsets, bass_tuning_name, bass_tuning_offsets, " + "rhythm_tuning_name, rhythm_tuning_offsets " + "FROM songs ") cursor = _decode_cursor(after) if after else None eff_sort = _effective_keyset_sort(sort, direction) if cursor and eff_sort in _KEYSET_SORTS: @@ -4071,8 +4267,30 @@ def query_page(self, q: str = "", page: int = 0, size: int = 24, "stem_ids": json.loads(r[12]) if r[12] else [], "tuning_name": r[13] or "", "tuning_offsets": r[14] or "", + # '' when the song has no bass arrangement (or the row predates + # '' when the song has no such chart (or the row predates the + # columns) — clients fall back to tuning_name. + "bass_tuning_name": r[15] or "", + "bass_tuning_offsets": r[16] or "", + "rhythm_tuning_name": r[17] or "", + "rhythm_tuning_offsets": r[18] or "", "has_estd": r[0] in estd, "favorite": r[0] in favs, }) + # PROVENANCE (non-default perspectives): a row shown to a bass or + # rhythm player either carries that chart's own tuning (native) or is + # borrowing the guitar-derived song tuning (inferred). The fallback is + # deliberate — a third of a real library has no bass chart and + # excluding it would be worse — but it must never be SILENT, or we + # reproduce the original bug in a new place. The client marks inferred + # rows; it can't infer this itself without duplicating the COALESCE. + # + # guitar-lead adds NOTHING here, so the default payload is unchanged. + _persp = _perspective(instrument) + if _persp.column_prefix: + _name_key = _persp.column("name") + for s in songs: + s["tuning_perspective"] = _persp.id + s["tuning_inferred"] = not s.get(_name_key) # Personal layer (difficulty + tags) rides along like `favorite`, so a # card can badge it without a second request. Notes stay OUT of the list # payload (they can be long) — fetch per-song via /user-meta. Batched to @@ -4169,7 +4387,7 @@ def _attach_display_charts(self, songs: list[dict], eff_by_fn: dict, rows = self.conn.execute( "SELECT mw.effective_work_key, m.filename, m.title, m.duration, m.tuning, " "m.arrangements, m.has_lyrics, m.mtime, m.format, m.stem_count, m.stem_ids, " - "m.tuning_name, m.tuning_offsets " + "m.tuning_name, m.tuning_offsets, m.bass_tuning_name, m.bass_tuning_offsets " "FROM songs m JOIN work_display mw ON mw.filename = m.filename " f"WHERE mw.effective_work_key IN ({ph}){intrinsic_frag} " "ORDER BY mw.is_group_representative DESC, m.mtime DESC, m.filename", @@ -4190,6 +4408,7 @@ def _attach_display_charts(self, songs: list[dict], eff_by_fn: dict, "stem_count": int(m[9] or 0), "stem_ids": json.loads(m[10]) if m[10] else [], "tuning_name": m[11] or "", "tuning_offsets": m[12] or "", + "bass_tuning_name": m[13] or "", "bass_tuning_offsets": m[14] or "", } def query_artists(self, letter: str = "", q: str = "", @@ -4204,7 +4423,9 @@ def query_artists(self, letter: str = "", q: str = "", stems_lacks: list[str] | None = None, has_lyrics: int | None = None, tunings: list[str] | None = None, - naming_mode: str = "legacy") -> tuple[list[dict], int]: + naming_mode: str = "legacy", + instrument: str = DEFAULT_PERSPECTIVE, + playable_from_pitch: int | None = None) -> tuple[list[dict], int]: """Get artists grouped by letter with their albums and songs. Returns (artists, total_artists).""" where, params = self._build_where( q=q, favorites_only=favorites_only, format_filter=format_filter, @@ -4212,6 +4433,7 @@ def query_artists(self, letter: str = "", q: str = "", arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode, + instrument=instrument, playable_from_pitch=playable_from_pitch, ) # Canonicalize artists at display when aliases exist (P4): dedupe / group / # letter / order on the EFFECTIVE artist so "ACDC" + "AC/DC" list as one @@ -4247,7 +4469,7 @@ def query_artists(self, letter: str = "", q: str = "", rows = self.conn.execute( f"SELECT filename, title, ({art_expr}) as artist, album, year, duration, tuning, arrangements, has_lyrics, " - f"format, stem_count, stem_ids, tuning_name " + f"format, stem_count, stem_ids, tuning_name, bass_tuning_name " f"FROM songs {song_where} ORDER BY ({art_expr}) COLLATE NOCASE, album COLLATE NOCASE, title COLLATE NOCASE", song_params ).fetchall() @@ -4280,6 +4502,7 @@ def query_artists(self, letter: str = "", q: str = "", "stem_count": int(r[10] or 0), "stem_ids": json.loads(r[11]) if r[11] else [], "tuning_name": r[12] or "", + "bass_tuning_name": r[13] or "", "has_estd": r[0] in estd, "favorite": r[0] in favs, "user_difficulty": udm.get(r[0]), @@ -4301,7 +4524,8 @@ def query_albums(self, q="", favorites_only=False, format_filter="", stems_has=None, stems_lacks=None, has_lyrics=None, tunings=None, mastery=None, match_states=None, genre=None, - naming_mode="legacy", page=0, size=120): + naming_mode="legacy", instrument=DEFAULT_PERSPECTIVE, + playable_from_pitch=None, page=0, size=120): """Distinct (artist, album) groups with a track count + a representative cover song, for the album-condensed browse (paged by album). Rows with no album name are excluded -- they can't form an album card. Same filters as @@ -4313,7 +4537,8 @@ def query_albums(self, q="", favorites_only=False, format_filter="", stems_has=stems_has, stems_lacks=stems_lacks, has_lyrics=has_lyrics, tunings=tunings, mastery=mastery, match_states=match_states, genre=genre, - naming_mode=naming_mode, + naming_mode=naming_mode, instrument=instrument, + playable_from_pitch=playable_from_pitch, ) awhere = where + " AND album IS NOT NULL AND album != ''" total = self.conn.execute( @@ -4344,7 +4569,9 @@ def query_stats(self, favorites_only: bool = False, sort: str = "artist", want_sort_letters: bool = False, group: bool = False, - naming_mode: str = "legacy") -> dict: + naming_mode: str = "legacy", + instrument: str = DEFAULT_PERSPECTIVE, + playable_from_pitch: int | None = None) -> dict: """Aggregate stats for the letter bar. Accepts the same filter params as query_page so the letter counts stay synchronized with the grid when filters are active. @@ -4371,7 +4598,8 @@ def query_stats(self, favorites_only: bool = False, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, has_lyrics=has_lyrics, tunings=tunings, match_states=match_states, - naming_mode=naming_mode, + naming_mode=naming_mode, instrument=instrument, + playable_from_pitch=playable_from_pitch, include_intrinsic=not group, ) if group: @@ -4383,7 +4611,8 @@ def query_stats(self, favorites_only: bool = False, "m", format_filter=format_filter, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode) + has_lyrics=has_lyrics, tunings=tunings, naming_mode=naming_mode, + instrument=instrument, playable_from_pitch=playable_from_pitch) mfrag, mparams = self._grouped_member_match(ifrag, iparams) where += mfrag params += mparams diff --git a/lib/routers/library.py b/lib/routers/library.py index 2b2a4964..c25e0251 100644 --- a/lib/routers/library.py +++ b/lib/routers/library.py @@ -19,7 +19,7 @@ import appstate from library_registry import ( - _library_filter_args, _sanitize_collection_rules, + _library_filter_args, _normalize_instrument, _sanitize_collection_rules, _safe_art_redirect_url, _split_csv, _sync_collection_provider, _unregister_collection_provider, ) @@ -52,7 +52,8 @@ def _require_library_provider_capability(provider: object, capability: str) -> N _OPTIONAL_NEW_PROVIDER_KWARGS = ("naming_mode", "sort", "want_sort_letters", "after", - "mastery", "match_states") + "mastery", "match_states", "instrument", + "playable_from_pitch") def _filter_provider_kwargs(method: object, kwargs: dict) -> dict: @@ -235,9 +236,20 @@ async def list_library(q: str = "", page: int = 0, size: int = 24, sort: str = " has_lyrics: str = "", tunings: str = "", provider: str = "local", mastery: str = "", tags: str = "", user_difficulty: str = "", match: str = "", genre: str = "", after: str = "", group: int = 0, - naming_mode: str = "legacy"): + naming_mode: str = "legacy", instrument: str = "", + tuning_match: str = "", playable_offsets: str = "", + playable_instrument: str = "", playable_string_count: str = ""): """Paginated library search through the selected library provider. + `instrument` is the tuning PERSPECTIVE ("guitar-lead" default | + "guitar-rhythm" | "bass"): which arrangement's tuning the tuning + filter/sort speaks for, with a guitar fallback when a song has no chart in + that role. + + `tuning_match=playable` switches the tuning filter from exact-match to + "playable without retuning" against the caller's current tuning + (`playable_offsets` + `playable_instrument` + `playable_string_count`). + `after` is an opaque keyset cursor (feedBack#636 item 3): pass back the `next_cursor` from the previous response to fetch the next page with a WHERE-seek instead of OFFSET. Providers that don't support it ignore it and @@ -270,7 +282,10 @@ async def list_library(q: str = "", page: int = 0, size: int = 24, sort: str = " artist=artist, album=album, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, + has_lyrics=has_lyrics, tunings=tunings, instrument=instrument, + tuning_match=tuning_match, playable_offsets=playable_offsets, + playable_instrument=playable_instrument, + playable_string_count=playable_string_count, ), ) # The cursor to resume after this page (effective sort folds in dir=desc). @@ -292,7 +307,7 @@ async def list_library_albums(q: str = "", page: int = 0, size: int = 120, stems_has: str = "", stems_lacks: str = "", has_lyrics: str = "", tunings: str = "", mastery: str = "", match: str = "", genre: str = "", - provider: str = "local"): + provider: str = "local", instrument: str = ""): """Album-condensed browse: distinct (artist, album) groups with a track count and a representative cover song. Paged by album. Same filters as /api/library.""" size = min(size, 500) @@ -306,7 +321,7 @@ async def list_library_albums(q: str = "", page: int = 0, size: int = 120, q=q, favorites=favorites, format=format, artist=artist, album=album, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, + has_lyrics=has_lyrics, tunings=tunings, instrument=instrument, ), ) return {"albums": albums, "total": total, "page": page, "size": size} @@ -319,7 +334,9 @@ async def list_artists(letter: str = "", q: str = "", favorites: int = 0, page: arrangements_has: str = "", arrangements_lacks: str = "", stems_has: str = "", stems_lacks: str = "", has_lyrics: str = "", tunings: str = "", provider: str = "local", - naming_mode: str = "legacy"): + naming_mode: str = "legacy", instrument: str = "", + tuning_match: str = "", playable_offsets: str = "", + playable_instrument: str = "", playable_string_count: str = ""): """Get artists grouped by letter with albums and songs (for tree view).""" size = min(size, 100) library_provider = _get_library_provider(provider) @@ -336,7 +353,7 @@ async def list_artists(letter: str = "", q: str = "", favorites: int = 0, page: artist=artist, album=album, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, + has_lyrics=has_lyrics, tunings=tunings, instrument=instrument, ), ) return {"artists": artists, "total_artists": total, "page": page, "size": size} @@ -350,7 +367,10 @@ async def library_stats(favorites: int = 0, q: str = "", format: str = "", has_lyrics: str = "", tunings: str = "", provider: str = "local", match: str = "", sort: str = "artist", sort_letters: int = 0, - group: int = 0, naming_mode: str = "legacy"): + group: int = 0, naming_mode: str = "legacy", + instrument: str = "", tuning_match: str = "", + playable_offsets: str = "", playable_instrument: str = "", + playable_string_count: str = ""): """Aggregate stats for the UI. Accepts the same filter params as /api/library so the letter bar mirrors the active grid filter set. `sort` selects the column the jump rail's `sort_letters` keys on; @@ -375,7 +395,10 @@ async def library_stats(favorites: int = 0, q: str = "", format: str = "", artist=artist, album=album, arrangements_has=arrangements_has, arrangements_lacks=arrangements_lacks, stems_has=stems_has, stems_lacks=stems_lacks, - has_lyrics=has_lyrics, tunings=tunings, + has_lyrics=has_lyrics, tunings=tunings, instrument=instrument, + tuning_match=tuning_match, playable_offsets=playable_offsets, + playable_instrument=playable_instrument, + playable_string_count=playable_string_count, ), ) @@ -407,14 +430,20 @@ def library_genres(provider: str = "local"): @router.get("/api/library/tuning-names") -async def list_tuning_names(provider: str = "local"): +async def list_tuning_names(provider: str = "local", instrument: str = ""): """Distinct tuning names present in the library, with per-tuning counts. Powers the tuning multi-select. Sorted by `tuning_sort_key` so names appear in the same musical order the sort uses - (feedBack#22) — E Standard first, then nearest neighbors.""" + (feedBack#22) — E Standard first, then nearest neighbors. + + `instrument=bass` groups by each song's bass-arrangement tuning + (guitar-derived fallback for songs without a bass chart) so bass + players see the tunings they'd actually play. Providers that predate + the kwarg simply don't receive it (signature-filtered).""" library_provider = _get_library_provider(provider) _require_library_provider_capability(library_provider, "library.read") - return await _call_library_provider_async(library_provider, "tuning_names") + return await _call_library_provider_async( + library_provider, "tuning_names", instrument=_normalize_instrument(instrument)) @router.get("/api/library/practice-suggestions") diff --git a/lib/scan.py b/lib/scan.py index 6780b04f..6e50046b 100644 --- a/lib/scan.py +++ b/lib/scan.py @@ -124,6 +124,29 @@ def _library_dirs(all_songs, dlc: Path) -> set[str]: return rels +def _has_unextracted_columns() -> bool: + """True while any `songs` row still carries NULL in a column added by an + additive migration — i.e. metadata the current extractor would fill but + that no existing row has yet (currently `bass_tuning_name`). + + The tree-signature fast path only asks "did the file set change"; on a + settled library the answer is no forever, so a schema addition would never + reach extraction. This one-row probe forces the full pass exactly until the + backfill completes — `put()` writes '' rather than NULL, so it self-clears + after the rescan instead of disabling the fast path permanently.""" + try: + from metadata_db import MetadataDB + cond = " OR ".join(f"{c} IS NULL" for c in MetadataDB._EXTRACTION_MARKER_COLS) + row = appstate.meta_db.conn.execute( + f"SELECT 1 FROM songs WHERE {cond} LIMIT 1").fetchone() + except Exception as e: + # A probe failure must not take the scan down; falling back to the fast + # path costs at most a delayed backfill. + log.debug("scan: unextracted-column probe failed: %s", e) + return False + return row is not None + + def _record_dir_signature(all_songs, dlc: Path) -> None: sig = _stat_dirs(dlc, _library_dirs(all_songs, dlc)) if sig is not None: # a dir vanished mid-scan → skip; next scan is full @@ -221,7 +244,7 @@ def background_scan(force: bool = False): # `force` (manual Refresh) always does the full pass. Seeding above is # idempotent — it only writes when a builtin is missing — so it does not # perturb the mtimes on a settled library. - if not force: + if not force and not _has_unextracted_columns(): stored = _load_dir_signature() if stored is not None and stored.get("dlc") == str(dlc): current = _stat_dirs(dlc, stored["dirs"].keys()) @@ -319,6 +342,15 @@ def _is_excluded_from_library(p: Path) -> bool: cached = None if not cached: to_scan.append((f, mtime, size, dlc)) + elif any(cached.get(c) is None for c in appstate.meta_db._EXTRACTION_MARKER_COLS): + # Row predates one of the per-perspective tuning columns (NULL + # from the additive migration), so that perspective's tuning was + # never extracted for it. Without this + # re-queue an existing library would keep every bass column empty + # forever — mtime/size still match, so nothing else would ever + # bring the row back through extraction. Converges: put() always + # writes '' (never NULL), so a rescanned row is never re-queued. + to_scan.append((f, mtime, size, dlc)) elif cached.get("arrangements") and any( "smart_name" not in a for a in cached["arrangements"] ): diff --git a/lib/scan_worker.py b/lib/scan_worker.py index c535c2d8..c388a2c6 100644 --- a/lib/scan_worker.py +++ b/lib/scan_worker.py @@ -27,7 +27,11 @@ from pathlib import Path from song import compute_smart_names -from tunings import tuning_name +from tunings import ( + DEFAULT_PERSPECTIVE, PERSPECTIVES, ROLE_PERSPECTIVES, normalize_offsets, + perspective_low_pitch, perspective_tuning_key, perspective_tuning_name, + tuning_name, +) import sloppak as sloppak_mod import loosefolder as loosefolder_mod @@ -43,6 +47,56 @@ def _relpath(f: Path, dlc: Path) -> str: return f.name +def _apply_role_tunings(meta: dict) -> None: + """Derive each ROLE perspective's tuning columns from the raw offsets the + extractor emitted (currently bass + rhythm; guitar-lead reads the + song-level columns the scanner has always written). + + The domain rules live in `tunings` (see the PERSPECTIVES table and the + block above it for the evidence behind each): + + 1. NORMALIZE FIRST. Stored bass arrays are commonly six elements whose + last two slots are padding, so bass truncates to four strings before + anything looks at them — padding must never reach the namer or the + grouping key. Guitar does NOT truncate (a 7-string array is real). + 2. Refuse to name data the perspective distrusts (bass up-tuning), so the + library can't send a player off to a tuning nobody plays. + 3. Group on CANONICAL PITCHES, not the raw offsets string — the same + physical tuning serialized two ways must be ONE facet entry. + + A song with no arrangement in that role gets EMPTY strings / 0, not NULL: + '' is the indexed "we looked, there is no such chart" state the library's + fallback keys on, while NULL means "never extracted" and re-scans. + """ + for persp in ROLE_PERSPECTIVES: + raw = meta.pop(f"{persp.role}_tuning_offsets", None) + offsets = normalize_offsets(raw, persp) + if offsets is None: + meta[persp.column("name")] = "" + meta[persp.column("sort_key")] = 0 + meta[persp.column("offsets")] = "" + meta[persp.column("key")] = "" + meta[persp.column("low_pitch")] = None + continue + meta[persp.column("name")] = perspective_tuning_name(offsets, persp) + meta[persp.column("sort_key")] = sum(offsets) + # The NORMALIZED offsets are what we store: padding is not data, and a + # client rendering target notes must not print phantom strings. + meta[persp.column("offsets")] = " ".join(str(o) for o in offsets) + meta[persp.column("key")] = perspective_tuning_key(offsets, persp) + meta[persp.column("low_pitch")] = perspective_low_pitch(offsets, persp) + + +def _apply_song_low_pitch(meta: dict, offsets: list[int]) -> None: + """Lowest open-string pitch of the SONG-level (guitar-lead) tuning, for + the "playable without retuning" comparison. Indexed here, on the existing + manifest-only pass — never by reopening chart JSON.""" + persp = PERSPECTIVES[DEFAULT_PERSPECTIVE] + norm = normalize_offsets(offsets, persp) + meta["tuning_low_pitch"] = ( + perspective_low_pitch(norm, persp) if norm is not None else None) + + def _extract_meta_sloppak(path: Path) -> dict: """Extract metadata for a sloppak (file or directory).""" meta = sloppak_mod.extract_meta(path) @@ -52,6 +106,8 @@ def _extract_meta_sloppak(path: Path) -> dict: meta["tuning_name"] = name meta["tuning_sort_key"] = sum(offsets) meta["tuning_offsets"] = " ".join(str(o) for o in offsets) + _apply_song_low_pitch(meta, offsets) + _apply_role_tunings(meta) meta["format"] = "sloppak" # `extract_meta` already populates `stem_ids` (feedBack#129); # default to empty for older callers / mocks. @@ -86,6 +142,8 @@ def _extract_meta_loosefolder(path: Path, dlc_root: Path | None) -> dict: meta["tuning_name"] = name meta["tuning_sort_key"] = sum(offsets) meta["tuning_offsets"] = " ".join(str(o) for o in offsets) + _apply_song_low_pitch(meta, offsets) + _apply_role_tunings(meta) meta["format"] = "loose" meta.setdefault("stem_ids", []) # The library helper exposes absolute filesystem paths for audio/art diff --git a/lib/sloppak.py b/lib/sloppak.py index 8f943e9a..838882c8 100644 --- a/lib/sloppak.py +++ b/lib/sloppak.py @@ -1240,6 +1240,27 @@ def _tuning_for_meta(arrangements_manifest: list[dict]) -> list[int]: return [0] * 6 +def _role_tuning_for_meta(arrangements_manifest: list[dict], role: str) -> list[int] | None: + """Per-ROLE companion to _tuning_for_meta: the tuning of the arrangement + playing `role` ("bass" / "rhythm"), or None when the pack has no such + arrangement with a tuning — the index then leaves that perspective's + columns empty and the library falls back to the song (guitar-first) + tuning, marking the row inferred. + + Exact name first, then a looser containment pass so an alt/bonus chart + ("Bass 2", "Alt Rhythm") still beats pretending the part is in the lead + guitar's tuning.""" + for match_exact in (True, False): + for entry in arrangements_manifest: + name = str(entry.get("name", "")).lower() + tun = entry.get("tuning") + if not (tun and isinstance(tun, list)): + continue + if name == role if match_exact else role in name: + return list(tun) + return None + + def extract_meta(path: Path) -> dict: """Fast metadata for the library scanner. Reads only the manifest.""" manifest = load_manifest(path) @@ -1262,6 +1283,10 @@ def extract_meta(path: Path) -> dict: has_lyrics = bool(manifest.get("lyrics")) tuning_offsets = _tuning_for_meta(arr_list) + # Per-role tunings alongside the song-level one, so the library can answer + # for whichever arrangement the player actually plays. + role_tunings = {f"{role}_tuning_offsets": _role_tuning_for_meta(arr_list, role) + for role in ("bass", "rhythm")} stems_list = manifest.get("stems", []) or [] valid_stems: list[dict] = [] @@ -1300,6 +1325,8 @@ def extract_meta(path: Path) -> dict: "disc": (lambda v: int(v) if str(v if v is not None else "").strip().isdigit() else None)(manifest.get("disc")), "duration": float(manifest.get("duration", 0) or 0), "tuning_offsets": tuning_offsets, # caller maps to a name via tunings.tuning_name + # None = the pack has no arrangement in that role. + **role_tunings, "arrangements": arrangements, "has_lyrics": has_lyrics, "stem_count": stem_count, diff --git a/lib/tunings.py b/lib/tunings.py index c5d34ed6..19460d26 100644 --- a/lib/tunings.py +++ b/lib/tunings.py @@ -416,27 +416,258 @@ def apply_flat_instrument_patch_to_profiles(cfg: dict, updates: dict) -> dict: }) return out +# ── Bass tuning normalization (library indexing) ───────────────────────────── +# +# Bass charts in the wild store SIX-element tuning arrays even when the chart is +# a 4-string part: slots 4-5 are PADDING. Confirmed by inspecting the charts +# themselves — across every pack whose bass and guitar tunings diverge, no bass +# note ever references string index 4 or 5 (the deepest reach is index 3). +# +# The feedpak spec carries NO string-count field (manifest `arrangement.tuning` +# is an untyped integer array, `minItems: 1`), and counting strings for real +# would mean parsing the 600KB-1.2MB arrangement JSON of every song on the +# manifest-only fast scan path — unacceptable for scan time. So we DEFAULT BASS +# TO 4 STRINGS and truncate. +# +# KNOWN GAP (deliberate, documented): a genuine 5- or 6-string bass is +# truncated to its low four. That is harmless for the overwhelmingly common +# case — a 5-string in standard truncates to [0,0,0,0] and still names +# "Standard" — and only misreads a tuning that DIFFERS at string 4 or above. +# Revisit if the spec ever gains a string count. +BASS_DEFAULT_STRING_COUNT = 4 + +# Bassists tune DOWN, essentially never up: a whole-instrument up-tune fights +# string tension. Anything above +1 semitone across the board is data we do not +# trust, not a tuning a human plays (the real-world example that motivated this +# is a bass array of [5,5,5,5,4,4] — "all four strings up a perfect fourth" — +# on a song whose guitar chart is dead standard and whose own note content is +# consistent with standard tuning; the offsets were almost certainly computed +# against a 6-string-bass reference with an uninitialised tail). +# +# Such a tuning MUST NOT be named: printing "A Standard" would send a player +# off to retune to something nobody plays. It degrades to the custom path, +# where it stays visible and distinct but makes no pitch claim. +BASS_MAX_PLAUSIBLE_OFFSET = 1 + + +# ── Tuning PERSPECTIVES ────────────────────────────────────────────────────── +# +# The library's tuning facet/filter/sort always answers for ONE arrangement +# role. There are three, matching `active_instrument_profile`: +# +# guitar-lead the song-level (guitar-first) tuning — the historical +# default. Its columns are the original unprefixed +# `tuning_*` family, so today's behaviour is byte-identical. +# guitar-rhythm the RHYTHM chart's own tuning. Lead and rhythm charts can +# disagree (the same bug a bassist hit, inside guitar). +# bass the BASS chart's own tuning. +# +# One table drives extraction, the derived columns, the SQL, and the labels — +# rather than three near-identical column families maintained in parallel. +class TuningPerspective: + __slots__ = ("id", "role", "instrument", "string_count", "column_prefix", + "truncate", "guard_up_tuning", "label") + + def __init__(self, id, role, instrument, string_count, column_prefix, + truncate, guard_up_tuning, label): + self.id = id + self.role = role # arrangement name to look for ('' = song-level) + self.instrument = instrument + self.string_count = string_count + self.column_prefix = column_prefix # '' | 'rhythm_' | 'bass_' + self.truncate = truncate + self.guard_up_tuning = guard_up_tuning + self.label = label + + @property + def instrument_key(self) -> str: + return instrument_key(self.instrument, self.string_count) + + def column(self, suffix: str) -> str: + return f"{self.column_prefix}tuning_{suffix}" + + +PERSPECTIVES: dict[str, TuningPerspective] = { + "guitar-lead": TuningPerspective( + "guitar-lead", "", "guitar", 6, "", False, False, "lead"), + "guitar-rhythm": TuningPerspective( + "guitar-rhythm", "rhythm", "guitar", 6, "rhythm_", False, False, "rhythm"), + # Bass alone truncates (padded arrays) and guards against up-tuned data — + # both are bass-specific findings, see the block above. + "bass": TuningPerspective( + "bass", "bass", "bass", BASS_DEFAULT_STRING_COUNT, "bass_", True, True, "bass"), +} + +DEFAULT_PERSPECTIVE = "guitar-lead" + +# Perspectives that carry their OWN indexed columns (guitar-lead reads the +# song-level ones, which the scanner has always written). +ROLE_PERSPECTIVES = tuple(p for p in PERSPECTIVES.values() if p.column_prefix) + + +def perspective(perspective_id) -> TuningPerspective: + """Resolve a perspective id, tolerating the legacy two-valued vocabulary + ('guitar' -> guitar-lead) and anything unknown (-> the default). An + unrecognised value must never change filter semantics.""" + if perspective_id in PERSPECTIVES: + return PERSPECTIVES[perspective_id] + if perspective_id == "guitar": + return PERSPECTIVES[DEFAULT_PERSPECTIVE] + return PERSPECTIVES[DEFAULT_PERSPECTIVE] + + +def normalize_offsets(offsets, persp: TuningPerspective) -> list[int] | None: + """Coerce a stored tuning array to the strings the perspective's + instrument actually has. Returns None for anything unusable (empty / + non-integer / too short), so callers leave the index empty rather than + record a guess.""" + if not isinstance(offsets, list) or not offsets: + return None + if any(isinstance(o, bool) for o in offsets): + return None + try: + vals = [int(o) for o in offsets] + except (TypeError, ValueError): + return None + if len(vals) < persp.string_count: + return None + # Only bass truncates: its arrays are padded (see above). A guitar array + # longer than 6 is a genuine 7/8-string chart, and cutting it to 6 would + # invent a tuning the chart does not have. + if persp.truncate: + return vals[:persp.string_count] + return vals + + +def offsets_are_plausible(offsets: list[int], persp: TuningPerspective) -> bool: + """False for data the perspective refuses to trust — currently only the + bass up-tuning guard (see BASS_MAX_PLAUSIBLE_OFFSET).""" + if not persp.guard_up_tuning: + return True + return all(o <= BASS_MAX_PLAUSIBLE_OFFSET for o in offsets) + + +def perspective_tuning_name(offsets: list[int], persp: TuningPerspective) -> str: + """Name a NORMALIZED tuning for this perspective, refusing to name data the + perspective distrusts — that becomes "Custom Tuning", which stays distinct + by its canonical pitches without asserting a tuning anyone plays.""" + if not offsets_are_plausible(offsets, persp): + return "Custom Tuning" + return tuning_name(offsets) + + +def perspective_tuning_key(offsets: list[int], persp: TuningPerspective) -> str: + """CANONICAL grouping key: the tuning's absolute open-string pitches, so + the same physical tuning groups as ONE facet entry no matter how it was + serialized. Keyed on pitch rather than the raw offsets string, which is + serialization-dependent and fragments. + + Joined with ':' and NOT ',' — this key travels back as a `tunings` filter + selector, and that query param is a COMMA-separated list, so a comma here + would be split into meaningless fragments and match nothing. + """ + midis = tuning_midis_from_offsets(persp.instrument_key, offsets) + if not midis: + return "" + return persp.id + ":" + ":".join(str(m) for m in midis) + + +def perspective_low_pitch(offsets: list[int], persp: TuningPerspective) -> int | None: + """Absolute MIDI pitch of the tuning's LOWEST open string — the value the + "playable without retuning" comparison is built on (see + `chart_is_playable_in`).""" + midis = tuning_midis_from_offsets(persp.instrument_key, offsets) + if not midis: + return None + return min(midis) + + +# ── "Playable without retuning" ────────────────────────────────────────────── +# +# What the player actually wants is "don't make me retune", not "match this +# label". A chart is playable as-is when every pitch it needs is reachable on +# the instrument as currently tuned. +# +# WHAT WE CAN HONESTLY COMPUTE. We index open-string TUNINGS, not the notes a +# chart plays — note data lives in the 600KB-1.2MB arrangement JSON, and the +# library scan is deliberately manifest-only, so we do not read it (indexing a +# per-song lowest note would mean opening every chart on every scan). +# +# So the comparison is on OPEN-STRING PITCH, with a conservative assumption: +# a chart may require its own lowest open string. That gives +# +# playable <=> your lowest open pitch <= the chart's lowest open pitch +# +# On a fretted instrument every pitch ABOVE your lowest open string is +# reachable by fretting (strings sit within an octave of each other and the +# neck gives ~2 octaves), so the low end is the binding constraint. This is +# exactly the dominant real case: a 5-string bass (low B) plays every 4-string +# standard chart AND every drop-D chart untouched, because the low D is just +# fretted on the B string. +# +# DELIBERATE LIMITATIONS, both erring toward NOT claiming playability: +# * A chart that never actually touches its lowest open string is excluded +# anyway. Conservative: excluding a playable chart costs a scroll; +# including an unplayable one costs a mid-practice retune, which is the +# failure this feature exists to prevent. +# * The UPPER bound is not checked — a chart tuned far above you could in +# principle exceed your neck. Checking it needs the note range we do not +# have. It is the rare direction (and the guard above already refuses +# up-tuned bass data), but it is a real gap, not an oversight. +def chart_is_playable_in(chart_low_pitch, your_low_pitch) -> bool: + """True when a chart whose lowest open string is `chart_low_pitch` needs no + retune for a player tuned to `your_low_pitch`. Unknown chart pitch => False + (never claim playability we cannot support).""" + if chart_low_pitch is None or your_low_pitch is None: + return False + return int(your_low_pitch) <= int(chart_low_pitch) + + +# Back-compat wrappers over the generic helpers — bass was the first +# perspective and reads better spelled out at bass-specific call sites. +def normalize_bass_offsets(offsets) -> list[int] | None: + return normalize_offsets(offsets, PERSPECTIVES["bass"]) + + +def bass_offsets_are_plausible(offsets: list[int]) -> bool: + return offsets_are_plausible(offsets, PERSPECTIVES["bass"]) + + +def bass_tuning_name(offsets: list[int]) -> str: + return perspective_tuning_name(offsets, PERSPECTIVES["bass"]) + + +def bass_tuning_key(offsets: list[int]) -> str: + return perspective_tuning_key(offsets, PERSPECTIVES["bass"]) + + def tuning_name(offsets: list[int]) -> str: - # All three pattern checks below are gated on `len(offsets) == 6`. The - # naming conventions here are 6-string-specific — e.g. a 7-string all-zeros + # The pattern checks below are gated on `len(offsets)` being 6 or 4. The + # naming conventions are E-standard-rooted — e.g. a 7-string all-zeros # tuning has a low B, not an E, so labeling it "E Standard" would be wrong. - # 7+-string community content falls through to the numeric fallback. See #43. - - # Standard tunings (all six strings same offset) + # 7+-string community content falls through to the numeric fallback (#43). + # + # Length 4 is accepted because a bass's open strings (EADG) are the low + # four of the guitar, so the same standard/drop names apply at the same + # offsets. Bass callers must normalize FIRST (`normalize_bass_offsets`): + # stored bass arrays are commonly six elements with a padded tail, and the + # padding must never reach this namer. See the block above. + + # Standard tunings (all strings same offset) standard = { 0: "E Standard", -1: "Eb Standard", -2: "D Standard", -3: "C# Standard", -4: "C Standard", -5: "B Standard", -6: "Bb Standard", -7: "A Standard", 1: "F Standard", 2: "F# Standard", } - if len(offsets) == 6 and all(o == offsets[0] for o in offsets): + if len(offsets) in (4, 6) and all(o == offsets[0] for o in offsets): name = standard.get(offsets[0]) if name: return name # Drop tunings (low string 2 semitones below the rest) # Named after the low string's note: e.g. offsets[-2,0,0,0,0,0] = Drop D (low E dropped to D) - if len(offsets) == 6 and offsets[0] == offsets[1] - 2 and all(o == offsets[1] for o in offsets[1:]): + if len(offsets) in (4, 6) and offsets[0] == offsets[1] - 2 and all(o == offsets[1] for o in offsets[1:]): note_names = ["E", "F", "F#", "G", "Ab", "A", "Bb", "B", "C", "C#", "D", "Eb"] low_note = note_names[offsets[0] % 12] return f"Drop {low_note}" diff --git a/static/js/library.js b/static/js/library.js index 1d358e51..d96cda94 100644 --- a/static/js/library.js +++ b/static/js/library.js @@ -410,6 +410,51 @@ function _applyLibraryProviderToParams(params) { return params; } +// ── Instrument-aware tuning (the bass-player tuning-filter report) ─────────── +// A song's bass chart is often tuned differently from its guitar chart, so the +// tuning facet, the `tunings` filter, the tuning sort and the row's tuning +// badge must all speak for the instrument the player actually plays. Read the +// host's working-tuning capability (the live selection, seeded from +// /api/settings at boot) rather than adding another settings fetch; hosts +// without the capability keep the guitar behaviour. +const _LIB_PERSPECTIVES = ['guitar-lead', 'guitar-rhythm', 'bass']; +let _libSettingsProfile = ''; + +export function _setLibraryProfile(profileId) { + _libSettingsProfile = _LIB_PERSPECTIVES.includes(profileId) ? profileId : ''; +} + +export function _libraryInstrument() { + // The PROFILE is the only three-valued source (lead / rhythm / bass); the + // working-tuning capability knows guitar-vs-bass but not lead-vs-rhythm, + // so it is only the fallback. + if (_libSettingsProfile) return _libSettingsProfile; + try { + const wt = window.feedBack?.workingTuning; + if (wt && typeof wt.get === 'function') { + const cur = wt.get(); + if (cur?.instrument === 'bass') return 'bass'; + } + } catch { /* capability absent/erroring — lead guitar is the safe default */ } + return 'guitar-lead'; +} + +export function _libraryInstrumentLabel() { + const p = _libraryInstrument(); + return p === 'bass' ? 'bass' : p === 'guitar-rhythm' ? 'rhythm' : 'lead'; +} + +// The tuning a row should SHOW: the bass chart's for a bass player, falling +// back to the song (guitar-derived) tuning when the song has no bass +// arrangement — the common case, not an edge path. +function _rowTuningRaw(song) { + const p = _libraryInstrument(); + const field = p === 'bass' ? 'bass_tuning_name' + : p === 'guitar-rhythm' ? 'rhythm_tuning_name' : ''; + if (field && song[field]) return song[field]; + return song.tuning || song.tuning_name || ''; +} + export function _resetLibraryProviderViewState() { L.libEpoch++; L.currentPage = 0; @@ -768,6 +813,8 @@ export function _applyLibFiltersToParams(params) { if (_libFilters.stemsLacks.length) params.set('stems_lacks', _libFilters.stemsLacks.join(',')); if (_libFilters.lyrics !== null) params.set('has_lyrics', String(_libFilters.lyrics)); if (_libFilters.tunings.length) params.set('tunings', _libFilters.tunings.join(',')); + // Which instrument's tuning the `tunings` filter + the tuning sort read. + if (_libraryInstrument() !== 'guitar-lead') params.set('instrument', _libraryInstrument()); return params; } @@ -851,6 +898,7 @@ async function _renderTuningList() { c.innerHTML = '
Loading...
'; try { const params = _applyLibraryProviderToParams(new URLSearchParams()); + params.set('instrument', _libraryInstrument()); const resp = await fetch(`/api/library/tuning-names?${params}`); if (!resp.ok) throw new Error(`HTTP ${resp.status}`); const data = await resp.json(); @@ -869,6 +917,11 @@ async function _renderTuningList() { fetchError = e.message || 'request failed'; } } + // NAME the perspective: silent instrument-following is the original bug in + // a new place — the user must be able to see which instrument these + // tunings describe. + const labelEl = document.getElementById('filter-tunings-label'); + if (labelEl) labelEl.textContent = `Tuning (${_libraryInstrumentLabel()})`; c.innerHTML = ''; if (fetchError) { c.innerHTML = `
Failed to load tunings (${esc(fetchError)}). Reopen the drawer to retry.
`; @@ -894,10 +947,17 @@ async function _renderTuningList() { const checked = _libFilters.tunings.includes(val); const row = document.createElement('label'); row.className = 'tuning-row'; + // Be honest about the fallback: songs with no bass arrangement borrow + // the guitar chart's tuning, and that must be visible rather than + // presented as a measured bass tuning. + const inferred = t.inferred_count || 0; + if (inferred) { + row.title = `${inferred} of ${t.count} inferred from the guitar chart (no bass arrangement)`; + } row.innerHTML = `` + `${esc(label)}` + - `${t.count}`; + `${t.count}${inferred ? ` (${inferred}~)` : ''}`; const cb = row.querySelector('input'); cb.onchange = () => { const i = _libFilters.tunings.indexOf(val); @@ -1244,6 +1304,10 @@ export function renderGridCards(songs, containerId = 'lib-grid', mode = 'replace const duration = song.duration ? formatTime(song.duration) : ''; const tuningRaw = song.tuning || song.tuning_name || ''; const tuning = displayTuningName(tuningRaw); + // The BADGE follows the player's instrument; `tuning` above stays the + // song's guitar-derived tuning because the retune action below rewrites + // the chart to E Standard and must not key on the bass part. + const tuningBadge = displayTuningName(_rowTuningRaw(song)); const artUrl = _librarySongArtUrl(song, providerId); const isLocalProvider = _isLocalLibraryProvider(providerId); const isSloppak = song.format === 'sloppak'; @@ -1299,7 +1363,7 @@ export function renderGridCards(songs, containerId = 'lib-grid', mode = 'replace
${(() => { const _nm = _getArrangementNamingMode(); return (song.arrangements || []).map(a => _arrangementBadgeHtml(a, _nm)).join(''); })()} - ${tuning ? `${esc(tuning)}` : ''} + ${tuningBadge ? `${esc(tuningBadge)}` : ''} ${song.has_lyrics ? `Lyrics` : ''} ${song.user_difficulty != null ? `◆${esc(song.user_difficulty)}` : ''} ${duration ? `${duration}` : ''} @@ -1470,6 +1534,9 @@ export async function renderTreeInto(containerId, countId, stats, letter, q, fav const duration = song.duration ? formatTime(song.duration) : ''; const tuningRaw = song.tuning || song.tuning_name || ''; const tuning = displayTuningName(tuningRaw); + // Badge follows the player's instrument; the retune action below + // keeps operating on the song's guitar-derived tuning. + const tuningBadge = displayTuningName(_rowTuningRaw(song)); const isLocalProvider = _isLocalLibraryProvider(providerId); const isSloppak = song.format === 'sloppak'; const stdRetune = isLocalProvider && localFilename && !isSloppak && tuningRaw && !song.has_estd && @@ -1496,8 +1563,8 @@ export async function renderTreeInto(containerId, countId, stats, letter, q, fav { const _nm = _getArrangementNamingMode(); for (const arrangement of (song.arrangements || [])) html += _arrangementBadgeHtml(arrangement, _nm); } - if (tuning) - html += `${esc(tuning)}`; + if (tuningBadge) + html += `${esc(tuningBadge)}`; if (song.has_lyrics) html += `Lyrics`; if (song.user_difficulty != null) diff --git a/static/js/settings.js b/static/js/settings.js index 7591524b..6fbaaac1 100644 --- a/static/js/settings.js +++ b/static/js/settings.js @@ -18,7 +18,7 @@ // back-import would close a cycle. player-controls keeps reading it through the host seam, and // app.js — the root, which imports both — wires it. That is exactly what the seam is for. import { hwcInitSettingsUI } from './highway-colors.js'; -import { _getArrangementNamingMode } from './library.js'; +import { _getArrangementNamingMode, _setLibraryProfile } from './library.js'; import { _applyMastery, _autoplayExitEnabled, _exitConfirmEnabled, _showUpNextEnabled, } from './player-controls.js'; @@ -111,6 +111,10 @@ export async function loadSettings() { if (dlcEl) dlcEl.value = data.dlc_dir || ''; _defaultArrangement = data.default_arrangement || ''; _syncDefaultArrangementSelect(_defaultArrangement); + // Feed the library its tuning PERSPECTIVE (lead / rhythm / bass) — the + // tuning facet, filter, sort and badges all answer for the profile the + // player actually plays. + _setLibraryProfile(data.active_instrument_profile); const pathwayEl = document.getElementById('setting-instrument-pathway'); if (pathwayEl) pathwayEl.value = _normalizeInstrumentPathway(data.pathway); const demucsEl = document.getElementById('demucs-server-url'); diff --git a/static/v3/index.html b/static/v3/index.html index 3dafcb17..6056f363 100644 --- a/static/v3/index.html +++ b/static/v3/index.html @@ -326,7 +326,7 @@

Filters

- Tuning + Tuning All tunings
diff --git a/static/v3/songs.js b/static/v3/songs.js index 255e8145..a12c6639 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -62,7 +62,7 @@ artist: '', album: '', grouping: true, // one card per song (multi-chart grouping); persisted - filters: { arr_has: [], arr_lacks: [], stem_has: [], stem_lacks: [], lyrics: '', tunings: [], mastery: [], match: [], genre: [] }, + filters: { arr_has: [], arr_lacks: [], stem_has: [], stem_lacks: [], lyrics: '', tunings: [], mastery: [], match: [], genre: [], tuningMatch: 'exact' }, page: 0, total: 0, loading: false, built: false, accuracy: {}, tuningNames: [], genres: [], artistCatalog: [], renderedHash: '', scrollBound: false, @@ -120,7 +120,7 @@ function activeFilterCount() { const f = state.filters; return f.arr_has.length + f.arr_lacks.length + f.stem_has.length + f.stem_lacks.length + - (f.lyrics ? 1 : 0) + f.tunings.length + (f.mastery ? f.mastery.length : 0) + + (f.lyrics ? 1 : 0) + (f.tuningMatch === 'playable' ? 1 : f.tunings.length) + (f.mastery ? f.mastery.length : 0) + (f.match ? f.match.length : 0) + (f.genre ? f.genre.length : 0) + (state.artist ? 1 : 0) + (state.album ? 1 : 0); } @@ -280,7 +280,15 @@ if (f.stem_has.length) p.set('stems_has', f.stem_has.join(',')); if (f.stem_lacks.length) p.set('stems_lacks', f.stem_lacks.join(',')); if (f.lyrics) p.set('has_lyrics', f.lyrics); - if (f.tunings.length) p.set('tunings', f.tunings.join(',')); + // The two modes answer different questions, so only one filters at a + // time: sending both would silently intersect them. + if (f.tunings.length && f.tuningMatch !== 'playable') p.set('tunings', f.tunings.join(',')); + // Which perspective the `tunings` filter + the tuning sort read. + if (libInstrument() !== 'guitar-lead') p.set('instrument', libInstrument()); + // "Playable without retuning": send the player's LIVE tuning and let the + // server do the pitch maths (the pitch tables live in lib/tunings.py — + // duplicating them here is how the two drift apart). + applyPlayableParams(p, f); if (f.mastery && f.mastery.length) p.set('mastery', f.mastery.join(',')); if (f.match && f.match.length) p.set('match', f.match.join(',')); if (f.genre && f.genre.length) p.set('genre', f.genre.join(',')); @@ -786,6 +794,10 @@ // so without them the chips render exactly as before. Decoration runs AFTER the // (sync) window paint so scrolling stays snappy; a token cancels a superseded pass. let _tuningDecorToken = 0; + // The instrument the current grid was queried/painted for, so a + // working-tuning change can tell a guitar<->bass SWITCH (re-query) from a + // retune within the same instrument (re-colour only). + let _lastRenderInstrument = null; function _applyChipMatch(chip, stateName) { chip.classList.remove('bg-fb-mid', 'bg-emerald-500', 'bg-amber-400'); chip.classList.add(stateName === 'match' ? 'bg-emerald-500' @@ -825,22 +837,31 @@ const shown = song.display_chart ? Object.assign({}, song, song.display_chart) : song; // In select mode the checkbox occupies top-2 left-2, so shift the // tuning chip right (left-9) to avoid overlapping it. + // Bass players see the bass chart's tuning (guitar fallback) — the card + // must agree with the facet/filter or the grid contradicts the pills. + const shownTuning = shownTuningName(shown); const tuningLabel = (typeof window.displayTuningName === 'function') - ? window.displayTuningName(shown.tuning_name || shown.tuning) - : (shown.tuning_name || ''); + ? window.displayTuningName(shownTuning) + : (shownTuning || ''); let tuning = ''; if (tuningLabel) { const rawOffsets = (typeof window.parseRawTuningOffsets === 'function') - ? (window.parseRawTuningOffsets(shown.tuning_offsets) - || window.parseRawTuningOffsets(shown.tuning_name || shown.tuning)) + ? (window.parseRawTuningOffsets(shownTuningOffsets(shown)) + || window.parseRawTuningOffsets(shownTuning)) : null; const targetNotes = (tuningLabel === 'Custom Tuning' && rawOffsets && typeof window.displayTuningTargets === 'function') ? window.displayTuningTargets(rawOffsets, { tuningName: tuningLabel }) : ''; - const badgeTitle = targetNotes + // Mark a tuning we INFERRED from the guitar chart (this song has no + // bass arrangement) so a bass player isn't shown a borrowed tuning + // as if it were their part's. `~` keeps the chip compact; the title + // spells it out. + const inferred = shown.tuning_inferred === true; + const badgeTitle = (targetNotes ? ('Custom Tuning: ' + targetNotes) - : tuningLabel; + : tuningLabel) + + (inferred ? ' — from the guitar chart (no bass arrangement)' : ''); const pos = 'absolute top-2 ' + (state.selectMode ? 'left-9' : 'left-2'); // Tag the chip with its offsets so decorateTuningChips() can colour it // green (matches your current tuning) / amber (needs a retune) after paint. @@ -848,8 +869,14 @@ // scores its bass tuning against the bass base pitches, not guitar — otherwise // a 4-string bass tuning read as guitar can false-match a guitar player. const chipArrs = shown.arrangements || []; - const chipIsBass = chipArrs.length > 0 - && chipArrs.every((a) => /\bbass\b/i.test((a && a.name) || '')); + // Bass either because the chip is SHOWING the bass chart's tuning + // (a bass player on a song that has one), or because every + // arrangement is a bass part. Checked via libInstrument() rather + // than comparing the two names — they are EQUAL for most songs, so + // a value comparison would flag a guitarist's chip as bass. + const chipIsBass = (libInstrument() === 'bass' && !!shown.bass_tuning_name) + || (chipArrs.length > 0 + && chipArrs.every((a) => /\bbass\b/i.test((a && a.name) || ''))); const matchAttr = (rawOffsets && rawOffsets.length) ? ' data-tuning-chip data-tuning-offsets="' + esc(rawOffsets.join(',')) + '"' + (chipIsBass ? ' data-tuning-bass="1"' : '') : ''; @@ -857,7 +884,7 @@ tuning = '' + esc('Custom Tuning') + '
' + esc(targetNotes) + ''; } else { - tuning = '' + esc(tuningLabel) + ''; + tuning = '' + esc(tuningLabel) + (inferred ? ' ~' : '') + ''; } } // Display-only (pointer-events-none) so a click falls through to the @@ -2489,6 +2516,89 @@ function _artistHostEl() { return document.getElementById('v3-songs-artistpage'); } + // ── Instrument-aware tuning (the bass-player tuning-filter report) ──────── + // A song's bass chart is often in a different tuning from its guitar chart, + // so the tuning facet/filter/sort and the card chip must speak for the + // instrument the player actually plays. The host's working-tuning + // capability already holds the live selection (seeded from /api/settings on + // boot, updated when the player switches) — read it rather than adding + // another settings fetch. `state.settingsInstrument` is the fallback for + // hosts where the capability isn't mounted. + // Three perspectives, matching `active_instrument_profile`: lead and rhythm + // guitar charts can be tuned differently too, so a rhythm player hits the + // same bug a bassist did. The PROFILE is the only three-valued source (the + // working-tuning capability knows guitar-vs-bass but not lead-vs-rhythm), + // so it wins; the capability is the live fallback for hosts where the + // profile hasn't loaded. + const PERSPECTIVES = ['guitar-lead', 'guitar-rhythm', 'bass']; + function libInstrument() { + if (PERSPECTIVES.indexOf(state.settingsProfile) >= 0) return state.settingsProfile; + try { + const wt = window.feedBack && window.feedBack.workingTuning; + if (wt && typeof wt.get === 'function') { + const cur = wt.get(); + if (cur && cur.instrument === 'bass') return 'bass'; + } + } catch (_) { /* capability absent/erroring — fall through to settings */ } + return state.settingsInstrument === 'bass' ? 'bass' : 'guitar-lead'; + } + + // "Playable without retuning" mode reads the player's CURRENT tuning from + // the working-tuning capability (the live session state the tuner writes), + // not a separate setting. No capability => we cannot know the current + // tuning, so the mode is unavailable rather than guessed. + function currentWorkingTuning() { + try { + const wt = window.feedBack && window.feedBack.workingTuning; + if (!wt || typeof wt.get !== 'function') return null; + const cur = wt.get(); + if (!cur || !Array.isArray(cur.offsets) || !cur.offsets.length) return null; + return cur; + } catch (_) { return null; } + } + + function playableAvailable() { return !!currentWorkingTuning(); } + + function applyPlayableParams(p, f) { + if (f.tuningMatch !== 'playable') return; + const cur = currentWorkingTuning(); + if (!cur) return; + p.set('tuning_match', 'playable'); + p.set('playable_offsets', cur.offsets.join(',')); + p.set('playable_instrument', cur.instrument === 'bass' ? 'bass' : 'guitar'); + p.set('playable_string_count', String(cur.stringCount || cur.offsets.length)); + } + + // Short human label for the perspective, for the facet/sort headers. + function libInstrumentLabel() { + const p = libInstrument(); + return p === 'bass' ? 'bass' : p === 'guitar-rhythm' ? 'rhythm' : 'lead'; + } + + // The column a row's tuning lives in for the active perspective. + function perspectiveTuningField() { + const p = libInstrument(); + return p === 'bass' ? 'bass_tuning_name' + : p === 'guitar-rhythm' ? 'rhythm_tuning_name' : ''; + } + + // The tuning a card should SHOW: bass players see the bass chart's tuning, + // falling back to the song (guitar-derived) tuning when the song has no + // bass arrangement — the common case, so the fallback is not an edge path. + function shownTuningName(song) { + const f = perspectiveTuningField(); + if (f && song[f]) return song[f]; + return song.tuning_name || song.tuning; + } + + function shownTuningOffsets(song) { + const f = perspectiveTuningField(); + if (f && song[f]) { + return song[f.replace('_name', '_offsets')] || song.tuning_offsets; + } + return song.tuning_offsets; + } + // Sync the two Settings gates into module state (fire-and-forget — the // cached flags gate entry-point rendering; openArtistPage re-checks). function refreshArtistPageGates() { @@ -2496,6 +2606,10 @@ if (!cfg) return; state.artistPagesEnabled = cfg.artist_pages_enabled !== false; state.artistLinksEnabled = cfg.artist_external_links === true; + // Fallback instrument for hosts without the working-tuning capability. + state.settingsInstrument = cfg.instrument === 'bass' ? 'bass' : 'guitar'; + // The three-valued perspective source (lead / rhythm / bass). + state.settingsProfile = cfg.active_instrument_profile || ''; }); } @@ -2809,12 +2923,13 @@ else if (s === 'has') lacksArr.push(value); // 'lacks' → cycles back to any (already removed) } - function triPill(group, value, label, st) { + function triPill(group, value, label, st, title) { const cls = st === 'has' ? 'bg-fb-good/30 text-fb-good border-fb-good/40' : st === 'lacks' ? 'bg-fb-low/30 text-fb-low border-fb-low/40' : 'bg-gray-800/50 text-fb-textDim border-gray-700'; const mark = st === 'has' ? '✓ ' : st === 'lacks' ? '✕ ' : ''; - return ''; + const tip = title ? ' title="' + esc(title) + '"' : ''; + return ''; } function renderDrawer() { const d = document.getElementById('v3-songs-drawer'); @@ -2834,7 +2949,32 @@ section('Match', [['review', 'To review'], ['matched', 'Matched'], ['unmatched', 'Unmatched'], ['pending', 'Not scanned']].map((it) => '').join('')) + // Genre facet — dynamic list from /api/library/genres (primary genre). (state.genres && state.genres.length ? section('Genre', state.genres.map((g) => '').join('')) : '') + - section('Tuning', (state.tuningNames || []).map((t) => { + // The facet header NAMES the perspective. Silent instrument-following + // is the original bug in a new place: the user must be able to tell + // which instrument these tunings describe. + section('Tuning (' + libInstrumentLabel() + ')', + // MODE toggle. Exact match answers "which tuning is this + // labelled"; Playable answers "will this cost me a retune" — + // which is what a player actually wants. Both are offered; + // exact stays the default so nothing changes unasked. + '
' + + [['exact', 'Exact tuning'], ['playable', 'Playable without retuning']].map((m) => { + const on = (f.tuningMatch || 'exact') === m[0]; + const dis = m[0] === 'playable' && !playableAvailable(); + return ''; + }).join('') + + '
' + + (f.tuningMatch === 'playable' + ? '
Charts you can play in your current tuning, no retune. Songs whose lowest string sits below yours are excluded.
' + : '') + + ((state.tuningNames || []).map((t) => { // Filter on the server's grouping key (raw offsets for customs) // so two "Custom Tuning" entries are distinct; show their target // notes in the label so they're distinguishable. @@ -2847,8 +2987,17 @@ const notes = offs ? window.displayTuningTargets(offs, { tuningName: t.name }) : ''; if (notes) label = 'Custom · ' + notes; } - return triPill('tuning', val, label + ' (' + t.count + ')', f.tunings.includes(val) ? 'has' : 'any'); - }).join('') || 'No tunings') + + // Be honest about the fallback: when some of a row's songs have + // no bass chart and are borrowing the guitar tuning, say so + // rather than presenting a borrowed tuning as a measured one. + const inf = t.inferred_count || 0; + const title = inf + ? inf + ' of ' + t.count + ' inferred from the guitar chart (no bass arrangement)' + : ''; + const countLabel = inf ? t.count + ', ' + inf + ' inferred' : String(t.count); + return triPill('tuning', val, label + ' (' + countLabel + ')', + f.tunings.includes(val) ? 'has' : 'any', title); + }).join('') || 'No tunings')) + // Multi-chart grouping toggle (P5e) — a VIEW mode, not a filter // (never counted in the badge, never saved into collection rules). // Local provider only: it's the one that implements group=. @@ -2878,6 +3027,11 @@ else if (g === 'tuning') { const i = f.tunings.indexOf(v); if (i >= 0) f.tunings.splice(i, 1); else f.tunings.push(v); } renderDrawer(); })); + d.querySelectorAll('[data-tuning-match]').forEach((b) => b.addEventListener('click', () => { + if (b.disabled) return; + f.tuningMatch = b.getAttribute('data-tuning-match'); + renderDrawer(); + })); d.querySelectorAll('[data-lyrics]').forEach((b) => b.addEventListener('click', () => { f.lyrics = b.getAttribute('data-lyrics'); renderDrawer(); })); d.querySelectorAll('[data-mastery]').forEach((b) => b.addEventListener('click', () => { const v = b.getAttribute('data-mastery'); const i = f.mastery.indexOf(v); if (i >= 0) f.mastery.splice(i, 1); else f.mastery.push(v); renderDrawer(); })); d.querySelector('[data-grouping]')?.addEventListener('click', () => { @@ -2891,7 +3045,7 @@ d.querySelector('[data-drawer-tidy]')?.addEventListener('click', openArtistTidyUp); d.querySelector('[data-drawer-close]')?.addEventListener('click', closeDrawer); d.querySelector('[data-drawer-clear]')?.addEventListener('click', async () => { - state.filters = { arr_has: [], arr_lacks: [], stem_has: [], stem_lacks: [], lyrics: '', tunings: [], mastery: [], match: [], genre: [] }; + state.filters = { arr_has: [], arr_lacks: [], stem_has: [], stem_lacks: [], lyrics: '', tunings: [], mastery: [], match: [], genre: [], tuningMatch: 'exact' }; state.artist = ''; state.album = ''; renderDrawer(); @@ -3478,13 +3632,15 @@ const providers = await loadProviders(); const [, tn] = await Promise.all([ (async () => { state.accuracy = (await jget('/api/stats/best')) || {}; })(), - jget('/api/library/tuning-names?provider=' + enc(state.provider)), + jget('/api/library/tuning-names?provider=' + enc(state.provider) + + '&instrument=' + enc(libInstrument())), loadArtistCatalog(), // Artist-page gates (PR-B) ride the initial fetch batch so the // first card paint already knows whether artist lines are links. refreshArtistPageGates(), ]); state.tuningNames = (tn && tn.tunings) || []; + _lastRenderInstrument = libInstrument(); try { const _g = await jget('/api/library/genres?provider=' + enc(state.provider)); state.genres = (_g && _g.genres) || []; } catch (e) { state.genres = []; } const opt = (arr, sel) => arr.map(([v, l]) => '').join(''); @@ -3512,7 +3668,13 @@ '' + '' + '
' + - '' + + // Name the perspective on the SORT too, not just the filter: tuning + // sort orders by musical distance from standard, and for a bass + // player that distance is measured on the bass tuning. Unlabelled, + // the grid silently reorders with no visible cause. + '' + '' + '' + '' + @@ -4130,6 +4292,23 @@ // visible tuning chips against the new tuning. Cheap: re-decorates in place, // no re-fetch or re-paint. No-op off the Songs grid or without the capability. sm.on('working-tuning-changed', () => { + // A guitar<->bass SWITCH changes which tuning the facet, the filter, + // the sort and the card chip speak for, so the grid must re-query — + // re-colouring chips would leave the guitar tuning on screen and a + // guitar-keyed filter applied. A retune within one instrument still + // takes the cheap in-place path below. + const inst = libInstrument(); + if (inst !== _lastRenderInstrument) { + _lastRenderInstrument = inst; + // A tuning selection keyed to the old instrument means nothing + // for the new one; clearing avoids an empty grid the user can't + // explain (the pills are re-rendered from the new facet). + state.filters.tunings = []; + const active = document.querySelector('.screen.active'); + if (active && active.id === 'v3-songs') reload(); + else _libraryDirty = true; + return; + } if (typeof songsActive === 'function' && !songsActive()) return; if (state.view !== 'grid') return; decorateTuningChips(_gridEl()); diff --git a/tests/js/v3_songs_tuning.test.js b/tests/js/v3_songs_tuning.test.js index 913f472f..604c4636 100644 --- a/tests/js/v3_songs_tuning.test.js +++ b/tests/js/v3_songs_tuning.test.js @@ -67,11 +67,27 @@ test('v3 songs.js uses display helpers for album-art tuning badge', () => { const src = fs.readFileSync(SONGS_JS, 'utf8'); // The card renderer's row variable was renamed song → shown when grouped // cards landed (the badge reads the representative chart); accept either. - assert.match(src, /displayTuningName\((?:song|shown)\.tuning_name \|\| (?:song|shown)\.tuning\)/); + // The raw read then moved behind shownTuningName() so the badge can answer + // for the active tuning perspective — accept that indirection too, and pin + // the fallback inside the helper below so this stays a real guard. + assert.match( + src, + /displayTuningName\((?:(?:song|shown)\.tuning_name \|\| (?:song|shown)\.tuning|shownTuning)\)/, + ); assert.match(src, /displayTuningTargets/); assert.match(src, /parseRawTuningOffsets/); }); +test('the tuning-perspective helper still falls back to tuning_name || tuning', () => { + // shownTuningName() is what the badge now reads. With no perspective field + // set (guitar-lead, the default) it must resolve exactly what the badge + // used to read inline, or guitar players silently lose their tuning label. + const src = fs.readFileSync(SONGS_JS, 'utf8'); + const body = src.match(/function shownTuningName\(song\)\s*\{[\s\S]*?\n {4}\}/); + assert.ok(body, 'shownTuningName() not found — the badge read moved again'); + assert.match(body[0], /return song\.tuning_name \|\| song\.tuning;/); +}); + test('raw offset tuning_name does not appear in rendered card HTML', () => { const html = renderSongCardBadge({ tuning_name: '-2 0 0 0 -2' }, helpers); assert.doesNotMatch(html, /-2 0 0 0 -2/); diff --git a/tests/test_library_tuning_instrument.py b/tests/test_library_tuning_instrument.py new file mode 100644 index 00000000..a0c331c6 --- /dev/null +++ b/tests/test_library_tuning_instrument.py @@ -0,0 +1,721 @@ +"""Instrument-aware tuning in the library (the KwasimodoZAZA bass report). + +A song's BASS chart is often tuned differently from its guitar chart, but the +library indexed exactly one guitar-first tuning per song — so a bass player +filtering "Drop D" got songs whose GUITAR is in Drop D, and playlists built +that way were wrong. + +These tests round-trip through the real extractors, the real scanner +derivation, the real SQLite schema/migration, and the real HTTP surface. The +only thing stubbed is metadata EXTRACTION in the scan tests (the production +process pool can't reach an in-process mock) — never the code under test. + +Real-library notes, all confirmed against actual pack contents: + +* Bass arrangements usually store SIX-element offset arrays even when the + chart is a 4-string part — slots 4-5 are PADDING (no bass chart in the + corpus references string index 4 or 5). So bass offsets are truncated to 4 + before naming or grouping. The feedpak spec has no string-count field, so 4 + is a documented default, not a read value. +* AC/DC "Girls Got Rhythm" stores [5,5,5,5,4,4] — every string up a fourth, + which no bassist plays. That is BAD DATA, and it must never be NAMED, or the + library sends a player to retune to a tuning that does not exist. +* Covet "Shibuya" (custom guitar tuning, dead-standard bass) is the headline + regression: the tester's bug in a single song. +""" + +import importlib +import json +import sys + +import pytest +import yaml +from fastapi.testclient import TestClient + +import sloppak as sloppak_mod +from scan_worker import _extract_meta_for_file +from tunings import ( + PERSPECTIVES, bass_offsets_are_plausible, bass_tuning_key, bass_tuning_name, + chart_is_playable_in, normalize_bass_offsets, perspective_tuning_key, + tuning_name, +) + + +# ── Fixtures ───────────────────────────────────────────────────────────────── + +@pytest.fixture() +def server_mod(tmp_path, monkeypatch): + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + sys.modules.pop("server", None) + mod = importlib.import_module("server") + yield mod + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + getattr(sys.modules.get("server"), "_join_background_db_threads", lambda: None)() + conn.close() + + +@pytest.fixture() +def client(server_mod): + c = TestClient(server_mod.app) + try: + yield c + finally: + c.close() + + +def _pack(root, name, arrangements): + """A directory-form pack whose manifest carries per-arrangement tunings.""" + d = root / name + d.mkdir(parents=True) + (d / "manifest.yaml").write_text(yaml.safe_dump({ + "title": name, "artist": "A", "duration": 100, + "arrangements": arrangements, "stems": [], + }), encoding="utf-8") + return d + + +def _put(server_mod, *, filename, title, tuning_name_="E Standard", + tuning_sort_key=0, tuning_offsets="0 0 0 0 0 0", + bass_tuning_name="", bass_tuning_sort_key=0, bass_tuning_offsets="", + bass_tuning_key=""): + server_mod.meta_db.put(filename, 1.0, 1, { + "title": title, "artist": "A", "album": "A - LP", "year": "2010", + "duration": 200.0, "tuning": tuning_name_, "arrangements": [], + "has_lyrics": False, "format": "sloppak", "stem_ids": [], + "tuning_name": tuning_name_, + "tuning_sort_key": tuning_sort_key, + "tuning_offsets": tuning_offsets, + "bass_tuning_name": bass_tuning_name, + "bass_tuning_sort_key": bass_tuning_sort_key, + "bass_tuning_offsets": bass_tuning_offsets, + "bass_tuning_key": bass_tuning_key, + }) + + +# ── 1. Extraction: sloppak ─────────────────────────────────────────────────── + +def test_sloppak_extract_indexes_both_tunings_when_they_differ(tmp_path): + """The reported case: guitar down a step, bass in standard. BOTH must be + indexed — previously only the guitar tuning survived.""" + d = _pack(tmp_path, "differ.sloppak", [ + {"name": "Lead", "tuning": [-2, 0, 0, -1, -2, 0]}, + {"name": "Bass", "tuning": [0, 0, 0, 0, 0, 0]}, + ]) + meta = sloppak_mod.extract_meta(d) + assert meta["tuning_offsets"] == [-2, 0, 0, -1, -2, 0] + assert meta["bass_tuning_offsets"] == [0, 0, 0, 0, 0, 0] + + +def test_sloppak_extract_leaves_bass_absent_without_bass_arrangement(tmp_path): + """No bass chart → None, NOT a copy of the guitar tuning. The library + falls back explicitly, so 'no bass part' stays distinguishable.""" + d = _pack(tmp_path, "nobass.sloppak", [ + {"name": "Lead", "tuning": [-2, -2, -2, -2, -2, -2]}, + {"name": "Rhythm", "tuning": [-2, -2, -2, -2, -2, -2]}, + ]) + meta = sloppak_mod.extract_meta(d) + assert meta["tuning_offsets"] == [-2, -2, -2, -2, -2, -2] + assert meta["bass_tuning_offsets"] is None + + +def test_sloppak_extract_bass_wins_over_guitar_first_ordering(tmp_path): + """The bass entry is listed FIRST in the manifest; the song tuning must + still be the guitar's while the bass column takes the bass entry — the two + selections are independent, not 'first wins'.""" + d = _pack(tmp_path, "order.sloppak", [ + {"name": "Bass", "tuning": [-4, -4, -4, -4, -4, -4]}, + {"name": "Lead", "tuning": [0, 0, 0, 0, 0, 0]}, + ]) + meta = sloppak_mod.extract_meta(d) + assert meta["tuning_offsets"] == [0, 0, 0, 0, 0, 0] + assert meta["bass_tuning_offsets"] == [-4, -4, -4, -4, -4, -4] + + +def test_sloppak_extract_ignores_bass_arrangement_without_a_tuning(tmp_path): + """A bass chart that authors no tuning gives us nothing to index; the + column stays empty rather than defaulting to a wrong all-zeros.""" + d = _pack(tmp_path, "untuned.sloppak", [ + {"name": "Lead", "tuning": [-2, -2, -2, -2, -2, -2]}, + {"name": "Bass"}, + ]) + assert sloppak_mod.extract_meta(d)["bass_tuning_offsets"] is None + + +def test_sloppak_extract_falls_back_to_an_alt_bass_chart(tmp_path): + """Only a "Bass 2" chart exists. Using it beats reporting the guitar + tuning as the player's bass tuning.""" + d = _pack(tmp_path, "altbass.sloppak", [ + {"name": "Lead", "tuning": [0, 0, 0, 0, 0, 0]}, + {"name": "Bass 2", "tuning": [-2, 0, 0, 0, 0, 0]}, + ]) + assert sloppak_mod.extract_meta(d)["bass_tuning_offsets"] == [-2, 0, 0, 0, 0, 0] + + +# ── 2. Scanner derivation (name / sort key / offsets string) ───────────────── + +def test_scan_worker_derives_bass_columns_like_the_guitar_ones(tmp_path): + """Guitar columns keep all six strings; bass columns are TRUNCATED to the + bass's four (the stored tail is padding — see tunings.normalize_bass_offsets).""" + d = _pack(tmp_path, "derive.sloppak", [ + {"name": "Lead", "tuning": [-2, -2, -2, -2, -2, -2]}, + {"name": "Bass", "tuning": [0, 0, 0, 0, 0, 0]}, + ]) + meta = _extract_meta_for_file(d) + assert meta["tuning_name"] == "D Standard" + assert meta["tuning_sort_key"] == -12 + assert meta["tuning_offsets"] == "-2 -2 -2 -2 -2 -2" + assert meta["bass_tuning_name"] == "E Standard" + assert meta["bass_tuning_sort_key"] == 0 + assert meta["bass_tuning_offsets"] == "0 0 0 0" + # Canonical key = absolute open pitches of a 4-string bass in standard. + assert meta["bass_tuning_key"] == "bass:28:33:38:43" + + +def test_bass_padding_is_truncated_before_naming_and_grouping(tmp_path): + """The padded tail must never reach the namer or the group key: a bass + stored six-wide and the same tuning stored four-wide must produce + IDENTICAL indexed columns.""" + six = _extract_meta_for_file(_pack(tmp_path, "six.sloppak", [ + {"name": "Bass", "tuning": [-2, 0, 0, 0, 0, 0]}])) + four = _extract_meta_for_file(_pack(tmp_path, "four.sloppak", [ + {"name": "Bass", "tuning": [-2, 0, 0, 0]}])) + for col in ("bass_tuning_name", "bass_tuning_offsets", + "bass_tuning_sort_key", "bass_tuning_key"): + assert six[col] == four[col], col + assert six["bass_tuning_name"] == "Drop D" + + +def test_scan_worker_bass_columns_empty_without_a_bass_arrangement(tmp_path): + """Empty string, never None: '' is the indexed 'we looked, no bass chart' + state, while NULL means 'never extracted' and triggers a re-scan.""" + d = _pack(tmp_path, "nobass2.sloppak", [{"name": "Lead", "tuning": [0] * 6}]) + meta = _extract_meta_for_file(d) + assert meta["bass_tuning_name"] == "" + assert meta["bass_tuning_sort_key"] == 0 + assert meta["bass_tuning_offsets"] == "" + + +def test_implausible_bass_tuning_is_never_named(tmp_path): + """Real library data, and it is BAD DATA: AC/DC "Girls Got Rhythm" stores + a bass tuning of [5,5,5,5,4,4] — every string up a perfect fourth, which + no bassist plays (roughly double string tension), on a song whose guitar + chart is dead standard. + + Truncation alone would leave [5,5,5,5] = "all strings up a 4th", which the + namer WOULD happily name. Naming it would send a player off to retune to a + tuning that does not exist, so the plausibility guard must refuse: bassists + tune down, essentially never up.""" + d = _pack(tmp_path, "weird.sloppak", [ + {"name": "Lead", "tuning": [0, 0, 0, 0, 0, 0]}, + {"name": "Bass", "tuning": [5, 5, 5, 5, 4, 4]}, + ]) + meta = _extract_meta_for_file(d) + assert meta["bass_tuning_name"] == "Custom Tuning" + assert meta["bass_tuning_offsets"] == "5 5 5 5" + assert meta["bass_tuning_sort_key"] == 20 + + +@pytest.mark.parametrize("offsets", [ + [5, 5, 5, 5], [5, 5, 5, 5, 4, 4], [2, 2, 2, 2], [12, 12, 12, 12], +]) +def test_up_tuned_bass_offsets_are_refused_by_the_guard(offsets): + """Anything above +1 semitone is data we do not trust. Note the namer + ALONE would name several of these ([2,2,2,2] -> "F# Standard"), which is + exactly the retune-to-nowhere the guard exists to prevent.""" + norm = normalize_bass_offsets(offsets) + assert bass_offsets_are_plausible(norm) is False + assert bass_tuning_name(norm) == "Custom Tuning" + + +@pytest.mark.parametrize("offsets,expected", [ + ([0, 0, 0, 0], "E Standard"), # standard + ([-1, -1, -1, -1], "Eb Standard"), # down a semitone + ([-2, 0, 0, 0], "Drop D"), # drop + ([1, 1, 1, 1], "F Standard"), # +1 is the plausible ceiling, still named +]) +def test_plausible_bass_tunings_are_still_named(offsets, expected): + """The guard must not over-fire: real down-tunings, standard, and the +1 + ceiling all keep their names.""" + assert bass_tuning_name(offsets) == expected + + +# ── 3. Storage round-trip + the pre-migration re-extract marker ────────────── + +def test_put_get_round_trips_the_bass_columns(server_mod): + _put(server_mod, filename="rt.sloppak", title="RT", + tuning_name_="D Standard", tuning_sort_key=-12, + tuning_offsets="-2 -2 -2 -2 -2 -2", + bass_tuning_name="E Standard", bass_tuning_offsets="0 0 0 0 0 0") + got = server_mod.meta_db.get("rt.sloppak", 1.0, 1) + assert got["tuning_name"] == "D Standard" + assert got["bass_tuning_name"] == "E Standard" + assert got["bass_tuning_offsets"] == "0 0 0 0 0 0" + + +def test_put_never_writes_null_bass_columns(server_mod): + """A freshly-scanned row is by definition extracted, so even a song with + no bass chart stores '' — otherwise it would look pre-migration forever + and the scanner would re-extract it on every single pass.""" + _put(server_mod, filename="fresh.sloppak", title="Fresh") + row = server_mod.meta_db.conn.execute( + "SELECT bass_tuning_name FROM songs WHERE filename = 'fresh.sloppak'").fetchone() + assert row[0] == "" + assert server_mod.meta_db.get("fresh.sloppak", 1.0, 1)["bass_tuning_name"] == "" + + +def test_pre_migration_row_reads_back_as_null(server_mod): + """A row written before the columns existed (simulated with raw SQL that + omits them) reads back None — the marker the scanner keys its re-extract + on. If this ever became '' the backfill would silently never run.""" + server_mod.meta_db.conn.execute( + "INSERT INTO songs (filename, mtime, size, title, artist, album, year, " + "duration, tuning, arrangements, has_lyrics, format, stem_count, " + "stem_ids, tuning_name, tuning_sort_key, tuning_offsets) " + "VALUES ('old.sloppak', 1.0, 1, 'Old', 'A', 'A - LP', '2010', 200.0, " + "'E Standard', '[]', 0, 'sloppak', 0, '[]', 'E Standard', 0, '0 0 0 0 0 0')") + server_mod.meta_db.conn.commit() + got = server_mod.meta_db.get("old.sloppak", 1.0, 1) + assert got["bass_tuning_name"] is None + # Same for the canonical key: coalescing this to '' would make the + # scanner's re-extract check unfireable and strand the backfill. + assert got["bass_tuning_key"] is None + + +def test_a_row_missing_only_the_canonical_key_still_re_extracts(server_mod): + """A row scanned by an EARLIER build of this feature has bass_tuning_name + but no bass_tuning_key. It must still be re-queued, or its custom tunings + would group on the old serialization-dependent key forever.""" + _put(server_mod, filename="halfway.sloppak", title="Halfway", + bass_tuning_name="Drop D", bass_tuning_offsets="-2 0 0 0") + server_mod.meta_db.conn.execute( + "UPDATE songs SET bass_tuning_key = NULL WHERE filename = 'halfway.sloppak'") + server_mod.meta_db.conn.commit() + cached = server_mod.meta_db.get("halfway.sloppak", 1.0, 1) + assert cached["bass_tuning_name"] == "Drop D" + assert cached["bass_tuning_key"] is None # → the scanner re-queues it + + +# ── 4. The migration actually backfills (the highest-risk gap) ─────────────── + +@pytest.fixture() +def scan_server(tmp_path, monkeypatch, isolate_logging, reset_scan_state): + """Server with the background scan forced in-process (see + test_feedpak_extension.py::scan_server — the production spawn pool can't + reach an in-process mock).""" + import concurrent.futures + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + monkeypatch.delenv("DLC_DIR", raising=False) + sys.modules.pop("server", None) + mod = importlib.import_module("server") + import scan as scan_mod + monkeypatch.setattr( + scan_mod, "_make_scan_executor", + lambda: concurrent.futures.ThreadPoolExecutor(max_workers=4), + ) + yield mod + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + getattr(sys.modules.get("server"), "_join_background_db_threads", lambda: None)() + conn.close() + + +def test_existing_library_backfills_bass_tuning_on_next_scan(tmp_path, scan_server): + """END TO END for every CURRENT user: a settled library whose rows predate + the bass columns must re-extract on the next scan. + + Both guards are exercised together — the row-level "bass column is NULL → + re-queue" AND the tree-signature fast path, which on an unchanged library + would otherwise skip the listing pass entirely and strand the backfill. + Then a second scan must NOT re-extract (the backfill converges, it doesn't + re-scan the whole library every launch). + """ + import unittest.mock as mock + + dlc = tmp_path / "dlc" + dlc.mkdir() + (dlc / "song.feedpak").write_bytes(b"") + # json.dumps, not %s: a Windows path interpolated raw produces invalid JSON + # escapes (\U, \d), the config silently fails to parse, and the scan then + # reports "no DLC folder configured" and extracts nothing. + (tmp_path / "config.json").write_text( + json.dumps({"dlc_dir": str(dlc)}), encoding="utf-8") + + scan = importlib.import_module("scan") + seen: list[str] = [] + + def mock_extract(f, dlc_dir): + seen.append(f.name) + return {"title": f.name, "artist": "A", "album": "", + "bass_tuning_name": "Drop D", "bass_tuning_sort_key": -2, + "bass_tuning_offsets": "-2 0 0 0 0 0"} + + with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract): + scan.background_scan() + assert "song.feedpak" in seen + + # Simulate the pre-migration state: the row exists and is otherwise + # fresh (mtime/size match), but its bass columns were never extracted. + scan.appstate.meta_db.conn.execute( + "UPDATE songs SET bass_tuning_name = NULL, bass_tuning_sort_key = NULL, " + "bass_tuning_offsets = NULL") + scan.appstate.meta_db.conn.commit() + + seen.clear() + scan.background_scan() + assert "song.feedpak" in seen, ( + "a row with NULL bass columns must re-extract — otherwise no " + "existing library ever gets the bass tuning") + + row = scan.appstate.meta_db.conn.execute( + "SELECT bass_tuning_name, bass_tuning_offsets FROM songs " + "WHERE filename = 'song.feedpak'").fetchone() + assert row == ("Drop D", "-2 0 0 0 0 0") + + # Converged: the fast path is back and nothing re-extracts. + seen.clear() + scan.background_scan() + assert seen == [] + + +# ── 5. The facet endpoint ──────────────────────────────────────────────────── + +@pytest.fixture() +def facet_seeded(server_mod): + """Three shapes, matching the real library's distribution: + differ — guitar D Standard, bass E Standard (the bug) + match — both Drop D (common) + nobass — guitar Drop D, no bass chart (fallback, common) + """ + _put(server_mod, filename="differ.sloppak", title="Differ", + tuning_name_="D Standard", tuning_sort_key=-12, + tuning_offsets="-2 -2 -2 -2 -2 -2", + bass_tuning_name="E Standard", bass_tuning_sort_key=0, + bass_tuning_offsets="0 0 0 0 0 0") + _put(server_mod, filename="match.sloppak", title="Match", + tuning_name_="Drop D", tuning_sort_key=-2, tuning_offsets="-2 0 0 0 0 0", + bass_tuning_name="Drop D", bass_tuning_sort_key=-2, + bass_tuning_offsets="-2 0 0 0 0 0") + _put(server_mod, filename="nobass.sloppak", title="NoBass", + tuning_name_="Drop D", tuning_sort_key=-2, tuning_offsets="-2 0 0 0 0 0") + + +def _facet(client, **kw): + return {t["name"]: t["count"] + for t in client.get("/api/library/tuning-names", params=kw).json()["tunings"]} + + +def test_facet_defaults_to_the_guitar_tuning(client, facet_seeded): + assert _facet(client) == {"D Standard": 1, "Drop D": 2} + + +def test_facet_bass_groups_by_bass_tuning_with_guitar_fallback(client, facet_seeded): + """differ counts under its BASS tuning (E Standard), match under Drop D, + and nobass — having no bass chart — falls back to its guitar Drop D rather + than vanishing from the facet.""" + assert _facet(client, instrument="bass") == {"E Standard": 1, "Drop D": 2} + + +def test_facet_ignores_an_unknown_instrument(client, facet_seeded): + """An unknown value must not silently change filter semantics.""" + assert _facet(client, instrument="theremin") == _facet(client) + + +# ── 6. The filter: the actual reported bug ─────────────────────────────────── + +def _files(client, **kw): + return {s["filename"] for s in client.get("/api/library", params=kw).json()["songs"]} + + +def test_bass_filter_excludes_a_song_whose_only_match_is_its_guitar_tuning( + client, facet_seeded): + """THE BUG. Filtering bass "D Standard" must NOT return `differ` — its + D Standard is the GUITAR chart; its bass is in E Standard.""" + assert _files(client, tunings="D Standard") == {"differ.sloppak"} + assert _files(client, tunings="D Standard", instrument="bass") == set() + + +def test_bass_filter_returns_songs_by_their_bass_tuning(client, facet_seeded): + """…and the converse: bass "E Standard" finds `differ`, which the guitar + filter would never return.""" + assert _files(client, tunings="E Standard") == set() + assert _files(client, tunings="E Standard", instrument="bass") == {"differ.sloppak"} + + +def test_bass_filter_keeps_songs_without_a_bass_arrangement_via_fallback( + client, facet_seeded): + """The most common shape. `nobass` has no bass chart, so it must still be + reachable under its guitar tuning instead of disappearing for bass users — + and the facet's count for that pill must equal what the filter returns.""" + got = _files(client, tunings="Drop D", instrument="bass") + assert got == {"match.sloppak", "nobass.sloppak"} + assert _facet(client, instrument="bass")["Drop D"] == len(got) + + +def test_custom_bass_tunings_stay_distinct_under_their_offsets(client, server_mod): + """Two unnameable bass tunings both label "Custom Tuning"; the facet keys + them on raw offsets so selecting one doesn't drag in the other. Uses the + real [5,5,5,5,4,4] shape from the library.""" + _put(server_mod, filename="c1.sloppak", title="C1", + bass_tuning_name="Custom Tuning", bass_tuning_sort_key=28, + bass_tuning_offsets="5 5 5 5 4 4") + _put(server_mod, filename="c2.sloppak", title="C2", + bass_tuning_name="Custom Tuning", bass_tuning_sort_key=-7, + bass_tuning_offsets="-3 -1 -1 -1 -1 0") + keys = [t["key"] for t in client.get( + "/api/library/tuning-names", params={"instrument": "bass"}).json()["tunings"] + if t["name"] == "Custom Tuning"] + assert sorted(keys) == sorted(["5 5 5 5 4 4", "-3 -1 -1 -1 -1 0"]) + assert _files(client, tunings="5 5 5 5 4 4", instrument="bass") == {"c1.sloppak"} + + +def test_stats_facet_counts_agree_with_the_bass_filter(client, facet_seeded): + """The A–Z rail / count surface must apply the same instrument-aware + predicate as the grid, or the header count contradicts the results.""" + body = client.get("/api/library/stats", params={ + "tunings": "Drop D", "instrument": "bass"}).json() + assert body["total_songs"] == 2 + + +# ── 7. Sort ────────────────────────────────────────────────────────────────── + +def test_tuning_sort_respects_the_instrument(client, facet_seeded): + """Tuning sort is musical distance from E Standard. For a bass player that + distance must be measured on the BASS tuning: `differ` is the furthest + song by guitar (D Standard, |−12|) but the nearest by bass (E Standard, 0), + so it moves from last to first.""" + def order(**kw): + return [s["filename"] for s in client.get( + "/api/library", params={"sort": "tuning", **kw}).json()["songs"]] + + guitar = order() + assert guitar[-1] == "differ.sloppak" + bass = order(instrument="bass") + assert bass[0] == "differ.sloppak" + + +# ── 8. Song payload ────────────────────────────────────────────────────────── + +# ── 9. Real-library offset SHAPES ──────────────────────────────────────────── +# Measured across the 59-pack test library: bass offset lists are NOT reliably +# 4 or reliably 6 — 41 store six elements, 1 stores four. Two six-element ones +# diverge in the tail (AC/DC "Girls Got Rhythm" [5,5,5,5,4,4]; Intervals +# "Libra" [-2,0,0,0,0,0]). Nothing may crash or mislabel on any of them. + +@pytest.mark.parametrize("offsets,expected", [ + ([0, 0, 0, 0], "E Standard"), # four-element (the 1 outlier) + ([0, 0, 0, 0, 0, 0], "E Standard"), # six-element all-equal (39 of them) + ([-1, -1, -1, -1], "Eb Standard"), # four-element, down a semitone + ([5, 5, 5, 5, 4, 4], "Custom Tuning"), # AC/DC — divergent tail + ([-2, 0, 0, 0, 0, 0], "Drop D"), # Intervals — drop + trailing zeros + ([0, 0, 0, 0, 0], "Custom Tuning"), # five: no naming convention → custom +]) +def test_real_library_bass_offset_shapes_name_without_crashing(offsets, expected): + assert tuning_name(offsets) == expected + + +@pytest.mark.parametrize("offsets", [ + [0, 0, 0, 0], [0, 0, 0, 0, 0, 0], [5, 5, 5, 5, 4, 4], [-2, 0, 0, 0, 0, 0], +]) +def test_real_library_bass_offset_shapes_survive_extraction(tmp_path, offsets): + """Each shape must round-trip the real extractor + scanner derivation, + landing on the NORMALIZED (truncated, plausibility-checked) columns.""" + norm = normalize_bass_offsets(offsets) + d = _pack(tmp_path, "shape.sloppak", [ + {"name": "Lead", "tuning": [0, 0, 0, 0, 0, 0]}, + {"name": "Bass", "tuning": offsets}, + ]) + meta = _extract_meta_for_file(d) + assert meta["bass_tuning_name"] == bass_tuning_name(norm) + assert meta["bass_tuning_offsets"] == " ".join(str(o) for o in norm) + assert meta["bass_tuning_sort_key"] == sum(norm) + assert meta["bass_tuning_key"] == bass_tuning_key(norm) + + +def test_named_bass_tunings_group_across_serialization_lengths(client, server_mod): + """The length question does NOT fragment NAMED tunings: a bass stored as + four elements and one stored as six both name "E Standard", and the facet + groups by name — so they land in ONE row with a combined count. This is the + common case (40 of the 42 bass arrangements in the real library).""" + _put(server_mod, filename="four.sloppak", title="Four", + bass_tuning_name=tuning_name([0, 0, 0, 0]), bass_tuning_offsets="0 0 0 0") + _put(server_mod, filename="six.sloppak", title="Six", + bass_tuning_name=tuning_name([0, 0, 0, 0, 0, 0]), + bass_tuning_offsets="0 0 0 0 0 0") + assert _facet(client, instrument="bass") == {"E Standard": 2} + assert _files(client, tunings="E Standard", instrument="bass") == { + "four.sloppak", "six.sloppak"} + + +def test_drop_d_bass_groups_across_serialization_lengths(client, server_mod): + """Same for the Intervals shape: [-2,0,0,0,0,0] and [-2,0,0,0] both name + "Drop D", so trailing zeros can't split a named tuning into two rows.""" + _put(server_mod, filename="d6.sloppak", title="D6", + bass_tuning_name=tuning_name([-2, 0, 0, 0, 0, 0]), + bass_tuning_sort_key=-2, bass_tuning_offsets="-2 0 0 0 0 0") + _put(server_mod, filename="d4.sloppak", title="D4", + bass_tuning_name=tuning_name([-2, 0, 0, 0]), + bass_tuning_sort_key=-2, bass_tuning_offsets="-2 0 0 0") + assert _facet(client, instrument="bass") == {"Drop D": 2} + + +def test_equivalent_custom_bass_tunings_group_into_one_facet_row(client, server_mod): + """Two CUSTOM bass tunings that are the same physical tuning must be ONE + facet row, however they were serialized. They group on canonical PITCHES + (bass_tuning_key), so the offsets string no longer fragments them — + previously this produced two rows with split counts.""" + key = bass_tuning_key([-3, -1, -1, -1]) + _put(server_mod, filename="c6.sloppak", title="C6", + bass_tuning_name="Custom Tuning", bass_tuning_sort_key=-6, + bass_tuning_offsets="-3 -1 -1 -1", bass_tuning_key=key) + _put(server_mod, filename="c4.sloppak", title="C4", + bass_tuning_name="Custom Tuning", bass_tuning_sort_key=-6, + bass_tuning_offsets="-3 -1 -1 -1", bass_tuning_key=key) + rows = client.get("/api/library/tuning-names", + params={"instrument": "bass"}).json()["tunings"] + customs = [t for t in rows if t["name"] == "Custom Tuning"] + assert len(customs) == 1 and customs[0]["count"] == 2 + assert _files(client, tunings=customs[0]["key"], instrument="bass") == { + "c6.sloppak", "c4.sloppak"} + + +def test_canonical_key_is_pitch_not_serialization(tmp_path): + """The property that makes the grouping robust: two serializations of one + tuning yield the same key, and two genuinely different tunings do not.""" + assert bass_tuning_key(normalize_bass_offsets([-2, 0, 0, 0, 0, 0])) == \ + bass_tuning_key(normalize_bass_offsets([-2, 0, 0, 0])) + assert bass_tuning_key([-2, 0, 0, 0]) != bass_tuning_key([-3, 0, 0, 0]) + # Absolute open pitches of a standard 4-string bass (E1 A1 D2 G2). + assert bass_tuning_key([0, 0, 0, 0]) == "bass:28:33:38:43" + + +def test_custom_bass_facet_row_selects_exactly_what_it_counted(client, server_mod): + """Whatever the grouping rule, the invariant that must NEVER break: every + facet row's count equals the number of songs its own key returns. This is + what makes the seam safe to change — a normalization that merged rows but + not the filter would fail here.""" + _put(server_mod, filename="x6.sloppak", title="X6", + bass_tuning_name="Custom Tuning", bass_tuning_sort_key=28, + bass_tuning_offsets="5 5 5 5 4 4") + _put(server_mod, filename="x4.sloppak", title="X4", + bass_tuning_name="Custom Tuning", bass_tuning_sort_key=20, + bass_tuning_offsets="5 5 5 5") + _put(server_mod, filename="plain.sloppak", title="Plain", + bass_tuning_name="E Standard", bass_tuning_offsets="0 0 0 0 0 0") + for row in client.get("/api/library/tuning-names", + params={"instrument": "bass"}).json()["tunings"]: + got = _files(client, tunings=row["key"], instrument="bass") + assert len(got) == row["count"], ( + f"facet row {row['key']!r} counted {row['count']} but selects {len(got)}") + + +# ── 10. THE HEADLINE REGRESSION ────────────────────────────────────────────── + +def test_covet_shibuya_is_findable_by_a_bassist(tmp_path, server_mod, client): + """Covet - "Shibuya" (Effloresce): the guitar is in a custom tuning + [-2,0,0,-1,-2,0] while the bass is dead standard. This is the tester's bug + in one song — a bassist filtering "E Standard" never saw it, because the + library only knew the guitar's custom tuning. + + Round-tripped through the REAL extractor and scanner derivation, not + hand-written columns, so it covers the whole chain.""" + d = _pack(tmp_path, "shibuya.sloppak", [ + {"name": "Lead", "tuning": [-2, 0, 0, -1, -2, 0]}, + {"name": "Bass", "tuning": [0, 0, 0, 0, 0, 0]}, + ]) + meta = _extract_meta_for_file(d) + server_mod.meta_db.put("shibuya.sloppak", 1.0, 1, { + **meta, "title": "Shibuya", "artist": "Covet", "album": "Effloresce"}) + + # The guitar chart really is a custom tuning… + assert meta["tuning_name"] == "Custom Tuning" + # …and the bass chart really is standard. + assert meta["bass_tuning_name"] == "E Standard" + + # Before the fix a bassist filtering E Standard got nothing. + assert _files(client, tunings="E Standard") == set() + assert _files(client, tunings="E Standard", instrument="bass") == {"shibuya.sloppak"} + + # And it appears in the bass facet under E Standard, as a REAL bass chart + # (not an inferred fallback). + row = next(t for t in client.get( + "/api/library/tuning-names", params={"instrument": "bass"}).json()["tunings"] + if t["name"] == "E Standard") + assert row["count"] == 1 and row["inferred_count"] == 0 + + +# ── 11. Provenance: the fallback must be honest, never silent ──────────────── + +def test_facet_reports_how_many_rows_are_inferred_from_the_guitar_chart( + client, facet_seeded): + """The fallback keeps no-bass-chart songs visible (a third of a real + library), but the UI must be able to say so. `nobass` has no bass chart and + rides under the guitar's Drop D; `match` has a real one.""" + rows = {t["name"]: t for t in client.get( + "/api/library/tuning-names", params={"instrument": "bass"}).json()["tunings"]} + assert rows["Drop D"]["count"] == 2 + assert rows["Drop D"]["inferred_count"] == 1 # nobass only + assert rows["E Standard"]["inferred_count"] == 0 # differ has a real bass chart + + +def test_guitar_facet_reports_no_inferred_rows(client, facet_seeded): + """Guitar is never a fallback perspective, so nothing is ever inferred.""" + rows = client.get("/api/library/tuning-names").json()["tunings"] + assert all(t["inferred_count"] == 0 for t in rows) + + +def test_song_rows_mark_an_inferred_tuning(client, facet_seeded): + """A bass player's row must be distinguishable: native bass chart vs + borrowed from the guitar. Without this the card silently presents a guitar + tuning as the bass tuning — the original bug in a new place.""" + rows = {s["filename"]: s for s in client.get( + "/api/library", params={"instrument": "bass"}).json()["songs"]} + assert rows["differ.sloppak"]["tuning_inferred"] is False + assert rows["nobass.sloppak"]["tuning_inferred"] is True + assert rows["differ.sloppak"]["tuning_perspective"] == "bass" + + +def test_guitar_rows_carry_no_bass_perspective_fields(client, facet_seeded): + """The guitar payload is untouched — no perspective/inferred keys at all.""" + row = client.get("/api/library").json()["songs"][0] + assert "tuning_inferred" not in row and "tuning_perspective" not in row + + +def test_arrangements_has_bass_is_the_real_bass_chart_lever(server_mod, client): + """'Only songs with a real bass chart' is the EXISTING `arrangements_has` + filter — no new filter, no "confirmed tunings" checkbox. It composes with + the tuning filter, so a bassist who wants to exclude inferred rows already + can, and it is already expressible in a saved collection rule.""" + def put_with_arrs(fn, arrs, **kw): + server_mod.meta_db.put(fn, 1.0, 1, { + "title": fn, "artist": "A", "album": "A - LP", "year": "2010", + "duration": 200.0, "tuning": "Drop D", "arrangements": arrs, + "has_lyrics": False, "format": "sloppak", "stem_ids": [], + "tuning_name": "Drop D", "tuning_sort_key": -2, + "tuning_offsets": "-2 0 0 0 0 0", **kw}) + + put_with_arrs("withbass.sloppak", + [{"index": 0, "name": "Lead"}, {"index": 1, "name": "Bass"}], + bass_tuning_name="Drop D", bass_tuning_sort_key=-2, + bass_tuning_offsets="-2 0 0 0", + bass_tuning_key=bass_tuning_key([-2, 0, 0, 0])) + put_with_arrs("nobass.sloppak", [{"index": 0, "name": "Lead"}]) + + # Both are reachable under the bass Drop D pill (the fallback keeps the + # no-bass-chart song visible)… + assert _files(client, tunings="Drop D", instrument="bass") == { + "withbass.sloppak", "nobass.sloppak"} + # …and the existing arrangements_has lever narrows to real bass charts. + assert _files(client, tunings="Drop D", instrument="bass", + arrangements_has="Bass") == {"withbass.sloppak"} + + +def test_song_rows_carry_the_bass_tuning_for_the_client(client, facet_seeded): + """The card renders the bass tuning client-side, so the row must ship it — + and ship '' (not the guitar value) when there is no bass chart, so the + client's fallback stays the client's decision.""" + rows = {s["filename"]: s for s in client.get("/api/library").json()["songs"]} + assert rows["differ.sloppak"]["tuning_name"] == "D Standard" + assert rows["differ.sloppak"]["bass_tuning_name"] == "E Standard" + assert rows["differ.sloppak"]["bass_tuning_offsets"] == "0 0 0 0 0 0" + assert rows["nobass.sloppak"]["bass_tuning_name"] == "" diff --git a/tests/test_library_tuning_perspectives.py b/tests/test_library_tuning_perspectives.py new file mode 100644 index 00000000..67ed6c23 --- /dev/null +++ b/tests/test_library_tuning_perspectives.py @@ -0,0 +1,294 @@ +"""The three-valued tuning PERSPECTIVE, and "playable without retuning". + +Two behaviours that extend the bass tuning fix (see +test_library_tuning_instrument.py): + +1. `active_instrument_profile` has three values (guitar-lead / guitar-rhythm / + bass), so the tuning perspective must too. Lead and rhythm charts can be + tuned differently, which is the identical bug a bassist hit, inside guitar. + +2. Exact tuning match answers "which tuning is this labelled". A player + actually wants "will this cost me a retune". Both are offered; exact stays + the default. + +Everything round-trips through the real extractor, the real scanner +derivation, the real schema and the real HTTP surface. +""" + +import importlib +import sys + +import pytest +import yaml +from fastapi.testclient import TestClient + +from scan_worker import _extract_meta_for_file +from tunings import ( + PERSPECTIVES, bass_tuning_key, chart_is_playable_in, perspective_tuning_key, +) + + +@pytest.fixture() +def server_mod(tmp_path, monkeypatch): + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + sys.modules.pop("server", None) + mod = importlib.import_module("server") + yield mod + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + getattr(sys.modules.get("server"), "_join_background_db_threads", lambda: None)() + conn.close() + + +@pytest.fixture() +def client(server_mod): + c = TestClient(server_mod.app) + try: + yield c + finally: + c.close() + + +def _pack(root, name, arrangements): + d = root / name + d.mkdir(parents=True) + (d / "manifest.yaml").write_text(yaml.safe_dump({ + "title": name, "artist": "A", "duration": 100, + "arrangements": arrangements, "stems": [], + }), encoding="utf-8") + return d + + +def _files(client, **kw): + return {s["filename"] for s in client.get("/api/library", params=kw).json()["songs"]} + + +# ── 1. The same bug WITHIN guitar: lead vs rhythm ──────────────────────────── + +def test_rhythm_chart_tuning_is_indexed_separately(tmp_path): + """A song whose LEAD is in E standard but whose RHYTHM is in Drop D must + index both — through the real extractor + scanner derivation.""" + d = _pack(tmp_path, "split.sloppak", [ + {"name": "Lead", "tuning": [0, 0, 0, 0, 0, 0]}, + {"name": "Rhythm", "tuning": [-2, 0, 0, 0, 0, 0]}, + ]) + meta = _extract_meta_for_file(d) + assert meta["tuning_name"] == "E Standard" # song-level = guitar-first + assert meta["rhythm_tuning_name"] == "Drop D" # the rhythm chart's own + assert meta["rhythm_tuning_offsets"] == "-2 0 0 0 0 0" + assert meta["rhythm_tuning_low_pitch"] == 38 # low D + + +def test_rhythm_offsets_are_not_truncated(tmp_path): + """Only BASS truncates (its arrays are padded). A 7-string guitar array is + real data — cutting it to 6 would invent a tuning the chart doesn't have.""" + d = _pack(tmp_path, "seven.sloppak", [ + {"name": "Rhythm", "tuning": [-2, -2, -2, -2, -2, -2, -2]}, + ]) + meta = _extract_meta_for_file(d) + assert meta["rhythm_tuning_offsets"] == "-2 -2 -2 -2 -2 -2 -2" + + +def test_no_rhythm_arrangement_leaves_the_columns_empty(tmp_path): + d = _pack(tmp_path, "leadonly.sloppak", [{"name": "Lead", "tuning": [0] * 6}]) + meta = _extract_meta_for_file(d) + assert meta["rhythm_tuning_name"] == "" + assert meta["rhythm_tuning_key"] == "" + + +def _put(server_mod, fn, **kw): + base = dict(title=fn, artist="A", album="LP", year="2010", duration=200.0, + tuning="E Standard", arrangements=[], has_lyrics=False, + format="sloppak", stem_ids=[], tuning_name="E Standard", + tuning_sort_key=0, tuning_offsets="0 0 0 0 0 0", + tuning_low_pitch=40) + base.update(kw) + server_mod.meta_db.put(fn, 1.0, 1, base) + + +@pytest.fixture() +def rhythm_seeded(server_mod): + """Both songs are E Standard by LEAD. One has a Drop D rhythm chart; the + other has no rhythm chart at all (so it falls back + is marked inferred).""" + _put(server_mod, "rdiffer.sloppak", + rhythm_tuning_name="Drop D", rhythm_tuning_sort_key=-2, + rhythm_tuning_offsets="-2 0 0 0 0 0", + rhythm_tuning_key=perspective_tuning_key( + [-2, 0, 0, 0, 0, 0], PERSPECTIVES["guitar-rhythm"]), + rhythm_tuning_low_pitch=38) + _put(server_mod, "rnone.sloppak") + + +def test_rhythm_filter_excludes_a_lead_only_tuning_match(client, rhythm_seeded): + """THE WITHIN-GUITAR BUG. Filtering rhythm "E Standard" must not return + rdiffer — that is its LEAD tuning; its rhythm chart is in Drop D.""" + assert _files(client, tunings="E Standard") == {"rdiffer.sloppak", "rnone.sloppak"} + # rnone has no rhythm chart, so it falls back to its lead tuning and stays. + assert _files(client, tunings="E Standard", instrument="guitar-rhythm") == { + "rnone.sloppak"} + assert _files(client, tunings="Drop D", instrument="guitar-rhythm") == { + "rdiffer.sloppak"} + # …and Drop D finds nothing from the lead perspective. + assert _files(client, tunings="Drop D") == set() + + +def test_rhythm_perspective_marks_inferred_rows(client, rhythm_seeded): + rows = {s["filename"]: s for s in client.get( + "/api/library", params={"instrument": "guitar-rhythm"}).json()["songs"]} + assert rows["rdiffer.sloppak"]["tuning_inferred"] is False + assert rows["rnone.sloppak"]["tuning_inferred"] is True + assert rows["rdiffer.sloppak"]["tuning_perspective"] == "guitar-rhythm" + + +def test_rhythm_facet_reports_inferred_portion(client, rhythm_seeded): + rows = {t["name"]: t for t in client.get( + "/api/library/tuning-names", + params={"instrument": "guitar-rhythm"}).json()["tunings"]} + assert rows["Drop D"]["count"] == 1 and rows["Drop D"]["inferred_count"] == 0 + assert rows["E Standard"]["count"] == 1 and rows["E Standard"]["inferred_count"] == 1 + + +def test_facet_row_selects_exactly_what_it_counted_for_rhythm(client, rhythm_seeded): + """The invariant that must hold for EVERY perspective.""" + for row in client.get("/api/library/tuning-names", + params={"instrument": "guitar-rhythm"}).json()["tunings"]: + got = _files(client, tunings=row["key"], instrument="guitar-rhythm") + assert len(got) == row["count"], row["key"] + + +def test_guitar_lead_is_byte_identical_to_the_legacy_default(client, rhythm_seeded): + """The majority path must not regress: the default payload gains no keys, + and the legacy two-valued vocabulary still resolves to it.""" + default = client.get("/api/library").json() + explicit = client.get("/api/library", params={"instrument": "guitar-lead"}).json() + legacy = client.get("/api/library", params={"instrument": "guitar"}).json() + assert default == explicit == legacy + row = default["songs"][0] + assert "tuning_inferred" not in row and "tuning_perspective" not in row + + +def test_unknown_perspective_falls_back_to_lead(client, rhythm_seeded): + """An unrecognised value must never silently change filter semantics.""" + assert client.get("/api/library", params={"instrument": "kazoo"}).json() == \ + client.get("/api/library").json() + + +def test_tuning_sort_respects_the_rhythm_perspective(client, rhythm_seeded): + """Sort is musical distance from standard. rdiffer is 0 away by lead but + -2 by rhythm, so the perspective changes its position.""" + def order(**kw): + return [s["filename"] for s in client.get( + "/api/library", params={"sort": "tuning", **kw}).json()["songs"]] + assert order()[0] == "rdiffer.sloppak" # tie → filename + assert order(instrument="guitar-rhythm")[0] == "rnone.sloppak" # 0 beats -2 + + +# ── 2. "Playable without retuning" ─────────────────────────────────────────── + +@pytest.mark.parametrize("your_low,chart_low,expected", [ + (23, 28, True), # 5-string bass (low B) plays a 4-string standard chart + (23, 26, True), # …and a drop-D chart: the low D is fretted on the B string + (28, 26, False), # 4-string standard CANNOT reach a drop-D chart's low D + (28, 28, True), # identical tuning + (40, 38, False), # guitar standard vs a drop-D chart + (38, 40, True), # a drop-D guitar covers a standard chart + (None, 28, False), # unknown chart pitch is never claimed playable + (28, None, False), +]) +def test_playability_rule(your_low, chart_low, expected): + """The core comparison as a property: your lowest open string vs the + chart's lowest required pitch. Unknown => not playable (conservative).""" + assert chart_is_playable_in(chart_low, your_low) is expected + + +@pytest.fixture() +def pitched(server_mod): + _put(server_mod, "std.sloppak", tuning_low_pitch=40) + _put(server_mod, "dropd.sloppak", tuning="Drop D", tuning_name="Drop D", + tuning_offsets="-2 0 0 0 0 0", tuning_sort_key=-2, tuning_low_pitch=38) + _put(server_mod, "dropc.sloppak", tuning="Drop C", tuning_name="Drop C", + tuning_offsets="-4 -2 -2 -2 -2 -2", tuning_sort_key=-14, tuning_low_pitch=36) + + +def _playable(client, offsets, instrument="guitar", sc=6, **kw): + return {s["filename"] for s in client.get("/api/library", params={ + "tuning_match": "playable", "playable_offsets": offsets, + "playable_instrument": instrument, "playable_string_count": str(sc), **kw, + }).json()["songs"]} + + +def test_playable_from_standard_excludes_lower_tuned_charts(client, pitched): + """In E standard you can play the standard chart, but the drop-D and + drop-C charts need a retune — exactly what the tester wants surfaced.""" + assert _playable(client, "0,0,0,0,0,0") == {"std.sloppak"} + + +def test_playable_from_drop_c_covers_everything_above_it(client, pitched): + """Tuned DOWN to drop C, every higher-tuned chart is reachable by fretting + — the dominant real case this feature exists for.""" + assert _playable(client, "-4,-2,-2,-2,-2,-2") == { + "std.sloppak", "dropd.sloppak", "dropc.sloppak"} + + +def test_playable_is_a_mode_not_a_replacement_for_exact(client, pitched): + """Exact match still works untouched, and returns something DIFFERENT from + playable — they answer different questions.""" + exact = {s["filename"] for s in client.get( + "/api/library", params={"tunings": "Drop D"}).json()["songs"]} + assert exact == {"dropd.sloppak"} + assert _playable(client, "-2,0,0,0,0,0") == {"std.sloppak", "dropd.sloppak"} + + +def test_playable_excludes_rows_with_no_indexed_pitch(client, server_mod, pitched): + """Conservative by construction: a chart whose low pitch we could not + compute is EXCLUDED, never assumed playable. Wrongly claiming playability + costs a mid-practice retune — the failure this feature prevents.""" + _put(server_mod, "unknown.sloppak", tuning_low_pitch=None) + assert "unknown.sloppak" not in _playable(client, "-4,-2,-2,-2,-2,-2") + # …but it is still reachable normally, so it isn't lost from the library. + assert any(s["filename"] == "unknown.sloppak" + for s in client.get("/api/library").json()["songs"]) + + +def test_malformed_playable_tuning_applies_no_filter(client, pitched): + """A tuning we cannot resolve must not silently claim everything is + playable OR that nothing is — it applies no filter at all.""" + everything = {s["filename"] for s in client.get("/api/library").json()["songs"]} + assert _playable(client, "not,a,tuning") == everything + assert _playable(client, "") == everything + # A string count that disagrees with the offsets is equally unusable. + assert _playable(client, "0,0,0,0", instrument="guitar", sc=6) == everything + + +def test_playable_respects_the_bass_perspective(client, server_mod): + """A 5-string bass (low B) can play a 4-string standard bass chart. The + comparison must run on the BASS tuning — this song's GUITAR chart is tuned + far lower, so reading the wrong column would flip the answer.""" + _put(server_mod, "bassy.sloppak", + tuning="Custom Tuning", tuning_name="Custom Tuning", + tuning_offsets="-4 -2 -2 -1 -2 0", tuning_sort_key=-11, + tuning_low_pitch=36, + bass_tuning_name="E Standard", bass_tuning_sort_key=0, + bass_tuning_offsets="0 0 0 0", + bass_tuning_key=bass_tuning_key([0, 0, 0, 0]), + bass_tuning_low_pitch=28) + # 5-string bass low B (23) <= the chart low E (28) → playable. + got = {s["filename"] for s in client.get("/api/library", params={ + "tuning_match": "playable", "playable_offsets": "0,0,0,0,0", + "playable_instrument": "bass", "playable_string_count": "5", + "instrument": "bass"}).json()["songs"]} + assert got == {"bassy.sloppak"} + # A 4-string bass tuned UP a semitone (low F, 29) cannot reach the low E. + got_up = {s["filename"] for s in client.get("/api/library", params={ + "tuning_match": "playable", "playable_offsets": "1,1,1,1", + "playable_instrument": "bass", "playable_string_count": "4", + "instrument": "bass"}).json()["songs"]} + assert got_up == set() + + +def test_playable_and_stats_agree(client, pitched): + """The count surface must apply the same predicate as the grid.""" + body = client.get("/api/library/stats", params={ + "tuning_match": "playable", "playable_offsets": "0,0,0,0,0,0", + "playable_instrument": "guitar", "playable_string_count": "6"}).json() + assert body["total_songs"] == 1 diff --git a/tests/test_loosefolder.py b/tests/test_loosefolder.py index 3ed514dd..4a0ca0f4 100644 --- a/tests/test_loosefolder.py +++ b/tests/test_loosefolder.py @@ -302,3 +302,44 @@ def test_extract_meta_uses_lead_tuning_when_bass_sorts_first(tmp_path): meta = loosefolder.extract_meta(tmp_path) assert meta["tuning_offsets"] == [0, 0, 0, 0, 0, 0] + # …and the bass chart's OWN tuning is indexed alongside it, so a bass + # player's library filter isn't answered with the guitar tuning. + assert meta["bass_tuning_offsets"] == [-4, -4, -4, -4, 0, 0] + + +def test_extract_meta_bass_tuning_absent_without_bass_arrangement(tmp_path): + """A folder with no bass chart leaves the bass tuning EMPTY (None) rather + than echoing the guitar tuning — the library then falls back explicitly, + and 'no bass part' stays distinguishable from 'bass part in E Standard'.""" + (tmp_path / "audio.wem").write_bytes(b"\0") + (tmp_path / "lead.xml").write_text(_LEAD_STD_XML, encoding="utf-8") + + meta = loosefolder.extract_meta(tmp_path) + assert meta["tuning_offsets"] == [0, 0, 0, 0, 0, 0] + assert meta["bass_tuning_offsets"] is None + + +def test_extract_meta_bass_tuning_matches_guitar_is_still_indexed(tmp_path): + """The COMMON case: bass and guitar in the same tuning. The bass column + must still be populated — an empty one would be read as 'no bass chart'.""" + (tmp_path / "audio.wem").write_bytes(b"\0") + _write_min_xml(tmp_path / "lead.xml", arrangement="Lead") + _write_min_xml(tmp_path / "bass.xml", arrangement="Bass") + + meta = loosefolder.extract_meta(tmp_path) + assert meta["bass_tuning_offsets"] == [0, 0, 0, 0, 0, 0] + + +def test_extract_meta_manifest_tuning_does_not_become_the_bass_tuning(tmp_path): + """A manifest `tuning_offsets` overrides the SONG tuning but says nothing + about which chart it describes, so it must never be mistaken for the bass + part's tuning — with no bass chart the bass column stays empty.""" + (tmp_path / "audio.wem").write_bytes(b"\0") + (tmp_path / "lead.xml").write_text(_LEAD_STD_XML, encoding="utf-8") + (tmp_path / "manifest.json").write_text(json.dumps({ + "tuning_offsets": [-2, -2, -2, -2, -2, -2], + }), encoding="utf-8") + + meta = loosefolder.extract_meta(tmp_path) + assert meta["tuning_offsets"] == [-2, -2, -2, -2, -2, -2] + assert meta["bass_tuning_offsets"] is None