mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-08-11 03:09:57 +00:00
fix(library): tuning filter answers for your instrument, not always guitar (#1003)
* fix(library): tuning filter answers for your instrument, not always guitar The library indexed exactly one tuning per song, chosen guitar-first (lead > rhythm > combo, bass only as a last resort), and nothing consulted the player's instrument. A bassist filtering by tuning was shown the guitar chart's tuning, so playlists built by tuning contained songs needing a retune. Reported by a tester building bass practice sets; Covet "Shibuya" is the clean case, with a custom guitar tuning over a standard bass chart. Indexes each arrangement role's own tuning and makes the facet, filter, sort and labels answer for one perspective. `guitar-lead` reads the original unprefixed columns and adds no payload keys, so the default response is unchanged. The same defect existed inside guitar -- lead and rhythm charts can disagree -- so perspective is three-valued (guitar-lead, guitar-rhythm, bass) driven by one PERSPECTIVES table rather than parallel column families. Songs with no chart for the perspective fall back to the song-level tuning rather than vanishing (18 of 59 packs in the test library have no bass chart), but the fallback is marked inferred in the facet counts and on the row instead of being silently coalesced. "Only real charts" reuses the existing `arrangements_has` filter rather than adding one. Bass-specific handling, from measured content: - Bass tuning arrays are padded to six entries; charts never reference string index 4 or 5. Truncated to four before naming and grouping. - Grouping uses a canonical open-pitch key, so [-2,0,0,0] and [-2,0,0,0,0,0] are one facet row instead of two. - Offsets above +1 semitone are refused a name. Bassists tune down, near never up; one pack ships [5,5,5,5,4,4] (A-D-G-C, unplayable, and its own notes sit in the song's real key under standard tuning). Naming that would send a player to retune to a tuning that does not exist. Rhythm deliberately does not truncate -- padding is a bass finding, and cutting a seven-string array would invent a tuning the chart lacks. Adds an opt-in `tuning_match=playable` mode alongside exact match: a chart is offered when your lowest open pitch is at or below its lowest open pitch, so a five-string bass covers four-string standard and drop-D with no retune. Open strings only -- note range is not indexed and the scan stays manifest-only -- so it fails conservative: unknown low pitch is excluded, and the upper bound is unchecked and documented rather than guessed. Existing installs would otherwise never populate: the tree-signature fast path reports "unchanged" forever on a settled library. Rows with NULL marker columns re-extract, and the fast path is disabled until that backfill converges (writes use '' rather than NULL, so it self-clears). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SFDokqh2H6mEjk1Kgbi6JW Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> * test(v3): accept the tuning-perspective indirection in the badge guard The album-art badge now reads shownTuningName(), so the source-pattern guard no longer matched the inline `tuning_name || tuning` form and CI went red. Accept the helper, and pin the helper's own fallback in a companion test so the guard still fails if a guitar player's tuning label is ever dropped. Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> --------- Signed-off-by: ChrisBeWithYou <christian.a.cowan@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
f0d9c3abc0
commit
cc75cb876a
+33
-1
@@ -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 background_scan(force: bool = False):
|
||||
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"]
|
||||
):
|
||||
|
||||
Reference in New Issue
Block a user