mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-22 12:52:29 +00:00
feat(playlists): flag songs that are not in your current tuning
Making the library's tuning filter instrument-aware does not repair playlists
already built under the old guitar-first behaviour. Those keep their
wrong-tuning songs, so a player still hits a surprise retune mid-practice and
reasonably concludes nothing was fixed.
Adds a per-playlist check: each row is marked against the player's current
tuning, with a summary ("3 of 24 songs are not in your tuning"), a filter to
show only those, and an explicit removal that lists every affected song by
title and states they stay in the library. Flagging is the feature -- nothing
is ever removed without being asked for, and removal reuses the existing
per-song DELETE rather than adding a bulk destructive endpoint.
Reuses the tuner capability's coverage report and `window.feedBack
.workingTuning`, the same pair the library cards already score against,
rather than introducing another source of truth.
Two deliberate departures:
- A coverage report reads "not covered" both for a real mismatch and for a
bail-out it could not evaluate. Only a report carrying an actual reason
counts as a mismatch; the rest render as unknown. This differs from the
library grid, which paints every not-covered song amber -- acceptable on a
grid, not on a hand-curated playlist where a false warning costs trust.
- With no tuning perspective available it makes no claim at all, rather than
defaulting to guitar and reproducing the original bug in a new place.
Playlist rows carry `tuning_offsets` and `bass_only`; a tuning *name* cannot
be scored, since two "Custom Tuning" rows are different tunings.
Fully correct once the instrument-aware tuning filter lands. That dependency
is confined to `rowTuningForCheck()` in static/v3/playlists.js, marked SEAM.
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>
This commit is contained in:
parent
cc75cb876a
commit
2f532a6957
@ -125,6 +125,24 @@ def _put_perspective_value(meta: dict, col: str):
|
||||
|
||||
# ── SQLite metadata cache ─────────────────────────────────────────────────────
|
||||
|
||||
def _arrangements_all_bass(raw) -> bool:
|
||||
"""True when EVERY arrangement on a chart is a bass part (raw ``arrangements``
|
||||
JSON, as stored). Mirrors the library grid's card rule: such a chart's tuning
|
||||
must be scored against bass base pitches, or a 4-string bass tuning read as
|
||||
guitar can false-match a guitarist. A chart with no arrangements is not bass.
|
||||
"""
|
||||
try:
|
||||
arrs = json.loads(raw) if raw else []
|
||||
except (ValueError, TypeError):
|
||||
return False
|
||||
if not isinstance(arrs, list) or not arrs:
|
||||
return False
|
||||
return all(
|
||||
isinstance(a, dict) and re.search(r"\bbass\b", str(a.get("name") or ""), re.I)
|
||||
for a in arrs
|
||||
)
|
||||
|
||||
|
||||
def _ensure_smart_names(arrangements: list[dict]) -> list[dict]:
|
||||
"""Fill in missing ``smart_name`` fields and sort arrangements by smart order.
|
||||
|
||||
@ -2697,7 +2715,7 @@ class MetadataDB:
|
||||
rows = self.conn.execute(
|
||||
f"""SELECT ps.filename, ps.position, s.title, s.artist, s.tuning_name,
|
||||
ps.arrangement, ps.work_key, s.arrangements,
|
||||
(s.filename IS NULL) AS dead
|
||||
(s.filename IS NULL) AS dead, s.tuning_offsets
|
||||
FROM playlist_songs ps LEFT JOIN songs s ON s.filename = ps.filename
|
||||
WHERE ps.playlist_id = ? {dead_filter}
|
||||
ORDER BY ps.position, ps.filename""",
|
||||
@ -2709,6 +2727,13 @@ class MetadataDB:
|
||||
entry = {
|
||||
"filename": r[0], "position": r[1],
|
||||
"title": r[2] or r[0], "artist": r[3] or "", "tuning_name": r[4] or "",
|
||||
# Offsets + the bass-only flag let the playlist tuning check score a
|
||||
# row against the player's working tuning the same way the library
|
||||
# grid's chips do: a NAME alone can't be scored (two "Custom Tuning"
|
||||
# rows are different tunings), and coverage needs to know whether to
|
||||
# measure against bass or guitar base pitches.
|
||||
"tuning_offsets": r[9] or "",
|
||||
"bass_only": _arrangements_all_bass(r[7]),
|
||||
"art_url": f"/api/song/{quote(r[0])}/art",
|
||||
}
|
||||
if is_album:
|
||||
@ -2736,7 +2761,8 @@ class MetadataDB:
|
||||
if work_key:
|
||||
self._ensure_work_display()
|
||||
row = self.conn.execute(
|
||||
"SELECT wd.filename, s.title, s.artist, s.tuning_name, s.arrangements "
|
||||
"SELECT wd.filename, s.title, s.artist, s.tuning_name, s.arrangements, "
|
||||
"s.tuning_offsets "
|
||||
"FROM work_display wd JOIN songs s ON s.filename = wd.filename "
|
||||
"WHERE wd.effective_work_key = ? AND wd.is_group_representative = 1",
|
||||
(work_key,)).fetchone()
|
||||
@ -2746,8 +2772,12 @@ class MetadataDB:
|
||||
arrs = _ensure_smart_names(json.loads(row[4]) if row[4] else [])
|
||||
except Exception:
|
||||
arrs = []
|
||||
# An orphan-resolved slot PLAYS a different chart, so it must report
|
||||
# that chart's tuning to the check — not the dead pin's.
|
||||
return {"resolved_filename": row[0], "title": row[1] or row[0],
|
||||
"artist": row[2] or "", "tuning_name": row[3] or "",
|
||||
"tuning_offsets": row[5] or "",
|
||||
"bass_only": _arrangements_all_bass(row[4]),
|
||||
"arrangements": arrs,
|
||||
"art_url": f"/api/song/{quote(row[0])}/art",
|
||||
"resolved_from_orphan": True}
|
||||
|
||||
@ -53,12 +53,185 @@
|
||||
return (m && m.index != null) ? m.index : null;
|
||||
}
|
||||
|
||||
// ── Playlist tuning check ────────────────────────────────────────────────
|
||||
// Playlists are commonly grouped BY TUNING so a practice run needs no
|
||||
// retune mid-session (retuning a bass is minutes of settling, and detuning
|
||||
// far on standard gauges goes floppy). A playlist built before the tuning
|
||||
// filter knew about your instrument can hold songs you can't actually play
|
||||
// without stopping. This flags them. It is READ-ONLY: nothing here edits a
|
||||
// playlist — removal is a separate, explicit, itemised action.
|
||||
|
||||
// ── SEAM — repoint HERE when `feat/v3-tuning-filter-instrument` lands ────
|
||||
// The tuning this row should be scored against for the player's instrument.
|
||||
// main indexes exactly ONE tuning per chart, derived from the guitar
|
||||
// arrangement, so today a bass player is scored against the guitar chart's
|
||||
// tuning — the very data source behind the reported bug. The sibling PR adds
|
||||
// per-instrument tuning columns (`bass_tuning_name` / `bass_tuning_offsets`)
|
||||
// plus a `libInstrument()` perspective; when it lands, this ONE function
|
||||
// picks the bass tuning for a bass player and everything downstream (the
|
||||
// chips, the summary, the filter, the remove list) follows with no other
|
||||
// change. Kept as a function, not an inline read, for exactly that reason.
|
||||
function rowTuningForCheck(s) {
|
||||
return {
|
||||
offsets: s.tuning_offsets || s.tuning_name,
|
||||
// Score a bass-only chart against bass base pitches — a 4-string bass
|
||||
// tuning read as guitar can false-match a guitarist.
|
||||
isBass: !!s.bass_only,
|
||||
};
|
||||
}
|
||||
|
||||
// A coverage report says "not covered" BOTH for a real mismatch and for
|
||||
// "I couldn't work it out" (missing settings/tuner data → an all-empty
|
||||
// report). Only a report carrying an actual reason — named string changes,
|
||||
// a reference-pitch gap, or too few strings — is a mismatch. An unexplained
|
||||
// not-covered is UNKNOWN. A false "wrong tuning" on a hand-curated playlist
|
||||
// costs more trust than saying nothing.
|
||||
function tuningStateFromReport(rep) {
|
||||
if (!rep) return 'unknown';
|
||||
if (rep.covered) return 'match';
|
||||
if (rep.cantCover || rep.reference
|
||||
|| (Array.isArray(rep.retune) && rep.retune.length)) return 'mismatch';
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
// Score every row. Returns null when the host exposes no tuning perspective
|
||||
// at all (no working-tuning capability / no tuner coverage) — the caller
|
||||
// then renders the playlist exactly as before rather than claiming anything.
|
||||
async function checkPlaylistTuning(songs) {
|
||||
const cov = window._tunerAutoOpen && window._tunerAutoOpen.coverageReport;
|
||||
const hasWT = window.feedBack && window.feedBack.workingTuning
|
||||
&& typeof window.feedBack.workingTuning.get === 'function';
|
||||
if (typeof cov !== 'function' || !hasWT) return null;
|
||||
const parse = window.parseRawTuningOffsets;
|
||||
const out = [];
|
||||
for (const s of songs || []) {
|
||||
const t = rowTuningForCheck(s);
|
||||
const offs = (typeof parse === 'function') ? parse(t.offsets) : null;
|
||||
if (!offs || !offs.length || offs.some((n) => !isFinite(n))) {
|
||||
out.push({ song: s, state: 'unknown' });
|
||||
continue;
|
||||
}
|
||||
let rep = null;
|
||||
try {
|
||||
rep = await cov({
|
||||
tuning: offs, stringCount: offs.length,
|
||||
arrangement: t.isBass ? 'Bass' : 'Lead',
|
||||
});
|
||||
} catch (_) { rep = null; }
|
||||
out.push({ song: s, state: tuningStateFromReport(rep) });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// Colour + a TEXT marker per state — unknown is deliberately neutral-and-
|
||||
// dimmed rather than amber, because "I couldn't check this" is a different
|
||||
// claim from "this is the wrong tuning" and must not read as the latter.
|
||||
function paintTuningChip(chip, state) {
|
||||
if (!chip) return;
|
||||
if (chip.dataset.baseTitle == null) chip.dataset.baseTitle = chip.getAttribute('title') || '';
|
||||
chip.classList.remove('bg-fb-mid', 'bg-emerald-500', 'bg-amber-400', 'opacity-60');
|
||||
chip.classList.add(state === 'match' ? 'bg-emerald-500'
|
||||
: state === 'mismatch' ? 'bg-amber-400' : 'bg-fb-mid');
|
||||
if (state === 'unknown') chip.classList.add('opacity-60');
|
||||
chip.setAttribute('title', chip.dataset.baseTitle + (state === 'match'
|
||||
? ' — matches your tuning'
|
||||
: state === 'mismatch' ? ' — needs a retune'
|
||||
: ' — no tuning data, not checked'));
|
||||
// Never signal by colour alone.
|
||||
const mark = state === 'mismatch' ? ' ⚠' : state === 'unknown' ? ' ?' : '';
|
||||
let m = chip.querySelector('[data-tuning-mark]');
|
||||
if (!m) {
|
||||
m = document.createElement('span');
|
||||
m.setAttribute('data-tuning-mark', '');
|
||||
chip.appendChild(m);
|
||||
}
|
||||
m.textContent = mark;
|
||||
}
|
||||
|
||||
function tuningSummaryHtml(results) {
|
||||
const total = results.length;
|
||||
if (!total) return '';
|
||||
const mism = results.filter((r) => r.state === 'mismatch').length;
|
||||
const unk = results.filter((r) => r.state === 'unknown').length;
|
||||
const box = 'mb-4 rounded-lg border px-3 py-2 text-sm flex flex-wrap items-center gap-x-3 gap-y-2 ';
|
||||
if (!mism) {
|
||||
return '<div class="' + box + 'border-fb-good/40 bg-fb-good/10 text-fb-good">' +
|
||||
'<span>✓ All ' + total + ' songs are in your tuning.</span>' +
|
||||
(unk ? '<span class="text-fb-textDim text-xs">' + unk + ' couldn\'t be checked (no tuning data).</span>' : '') +
|
||||
'</div>';
|
||||
}
|
||||
return '<div class="' + box + 'border-amber-400/40 bg-amber-400/10 text-fb-text">' +
|
||||
'<span><strong>' + mism + '</strong> of ' + total + ' songs aren\'t in your tuning.</span>' +
|
||||
(unk ? '<span class="text-fb-textDim text-xs">' + unk + ' couldn\'t be checked (no tuning data) — left alone.</span>' : '') +
|
||||
'<span class="flex-1"></span>' +
|
||||
'<button id="v3-pl-tune-only" class="text-xs px-2 py-1 rounded border border-fb-border text-fb-textDim hover:text-fb-text" aria-pressed="false">Show only these</button>' +
|
||||
'<button id="v3-pl-tune-remove" class="text-xs px-2 py-1 rounded border border-fb-accent/50 text-fb-accent hover:bg-fb-accent/10">Remove them…</button>' +
|
||||
'</div>';
|
||||
}
|
||||
|
||||
// Run the check and wire its affordances. Read-only: the only mutation is
|
||||
// the explicit, itemised, confirmed removal below.
|
||||
async function applyTuningCheck(root, pl, pid, rerender) {
|
||||
const host = root.querySelector('#v3-pl-tuning');
|
||||
const listEl = root.querySelector('#v3-pl-songs');
|
||||
if (!host || !listEl) return;
|
||||
const results = await checkPlaylistTuning(pl.songs);
|
||||
if (!results) return; // no perspective → say nothing
|
||||
const rows = listEl.querySelectorAll('li[data-fn]');
|
||||
results.forEach((r, i) => {
|
||||
const li = rows[i];
|
||||
if (!li) return;
|
||||
li.setAttribute('data-tuning-state', r.state);
|
||||
paintTuningChip(li.querySelector('[data-tuning-chip]'), r.state);
|
||||
});
|
||||
host.innerHTML = tuningSummaryHtml(results);
|
||||
|
||||
const onlyBtn = host.querySelector('#v3-pl-tune-only');
|
||||
onlyBtn?.addEventListener('click', () => {
|
||||
const on = onlyBtn.getAttribute('aria-pressed') !== 'true';
|
||||
onlyBtn.setAttribute('aria-pressed', on ? 'true' : 'false');
|
||||
onlyBtn.textContent = on ? 'Show all' : 'Show only these';
|
||||
rows.forEach((li) => {
|
||||
li.classList.toggle('hidden', on && li.getAttribute('data-tuning-state') !== 'mismatch');
|
||||
});
|
||||
});
|
||||
|
||||
host.querySelector('#v3-pl-tune-remove')?.addEventListener('click', async () => {
|
||||
// Name every song BEFORE removing anything — a curated playlist is
|
||||
// user data, so the confirm has to be a list, not a count.
|
||||
const doomed = results.filter((r) => r.state === 'mismatch').map((r) => r.song);
|
||||
if (!doomed.length) return;
|
||||
const names = doomed.map((s) => '<li>' + esc(s.title || s.filename) + '</li>').join('');
|
||||
const msg = 'Remove these ' + doomed.length + ' song' + (doomed.length === 1 ? '' : 's')
|
||||
+ ' from "' + esc(pl.name) + '"?'
|
||||
+ '<ul class="mt-2 text-xs list-disc list-inside max-h-48 overflow-auto">' + names + '</ul>'
|
||||
+ '<p class="text-xs text-fb-textDim mt-2">They stay in your library — only this playlist changes, and you can add them back.</p>';
|
||||
const ok = (typeof window.uiConfirm === 'function')
|
||||
? await window.uiConfirm({
|
||||
title: 'Remove mismatched songs?', html: msg,
|
||||
confirmText: 'Remove ' + doomed.length, cancelText: 'Cancel', danger: true,
|
||||
})
|
||||
: window.confirm('Remove ' + doomed.length + ' song(s) from "' + pl.name + '"?\n\n'
|
||||
+ doomed.map((s) => '• ' + (s.title || s.filename)).join('\n')
|
||||
+ '\n\nThey stay in your library.');
|
||||
if (!ok) return;
|
||||
for (const s of doomed) {
|
||||
await fetch('/api/playlists/' + pid + '/songs/' + encodeURIComponent(s.filename),
|
||||
{ method: 'DELETE' });
|
||||
}
|
||||
rerender();
|
||||
});
|
||||
}
|
||||
|
||||
function songRow(s, opts) {
|
||||
opts = opts || {};
|
||||
const handle = opts.draggable
|
||||
? '<span class="cursor-grab text-fb-textDim/60 px-1" title="Drag to reorder">⠿</span>' : '';
|
||||
// The chip carries its own tuning so the post-paint check can colour it
|
||||
// in place (green = play it now, amber = needs a retune, dimmed ? =
|
||||
// couldn't tell) without re-rendering the list.
|
||||
const tuning = s.tuning_name
|
||||
? '<span class="ml-2 text-[0.625rem] bg-fb-mid text-black font-bold px-1.5 py-0.5 rounded-sm">' + esc(s.tuning_name) + '</span>' : '';
|
||||
? '<span data-tuning-chip class="ml-2 text-[0.625rem] bg-fb-mid text-black font-bold px-1.5 py-0.5 rounded-sm" title="' + esc(s.tuning_name) + '">' + esc(s.tuning_name) + '</span>' : '';
|
||||
// ── Curated-album slot extras (P6) — mixes/saved emit none of this ──
|
||||
// A slot plays its RESOLVED chart (data-play-fn: the pinned file, or
|
||||
// the work's current keeper when the pinned file is gone) with its
|
||||
@ -272,6 +445,9 @@
|
||||
'</div>' +
|
||||
'</div>' +
|
||||
meter +
|
||||
// Filled in after paint by applyTuningCheck (async, feature-detected)
|
||||
// — stays empty when the host exposes no tuning perspective.
|
||||
(pl.songs.length ? '<div id="v3-pl-tuning"></div>' : '') +
|
||||
(pl.songs.length
|
||||
? '<ul id="v3-pl-songs" class="space-y-1">' + pl.songs.map((s) => songRow(s, { draggable: !isSystem, album: isAlbum, acc: isAlbum ? slotAcc(s) : undefined })).join('') + '</ul>'
|
||||
: '<p class="text-fb-textDim">Empty — add songs from the library' + (isAlbum ? ' (the ⋮ menu or the batch bar\'s "Add to playlist")' : '') + '.</p>') +
|
||||
@ -321,6 +497,9 @@
|
||||
});
|
||||
const listEl = root.querySelector('#v3-pl-songs');
|
||||
if (listEl) wireSongRows(listEl, pid, () => renderPlaylistDetail(pid));
|
||||
// Post-paint so the list is interactive immediately; a per-song coverage
|
||||
// call can await the tuner plugin's settings fetch.
|
||||
if (listEl) applyTuningCheck(root, pl, pid, () => renderPlaylistDetail(pid));
|
||||
// Album slot editor (▾ per row): pick the slot's chart + arrangement.
|
||||
if (listEl && isAlbum) {
|
||||
listEl.querySelectorAll('li[data-fn]').forEach((li) => {
|
||||
|
||||
304
tests/js/v3_playlist_tuning_check.test.js
Normal file
304
tests/js/v3_playlist_tuning_check.test.js
Normal file
@ -0,0 +1,304 @@
|
||||
// The playlist tuning check (static/v3/playlists.js).
|
||||
//
|
||||
// A bass-playing tester built playlists grouped BY TUNING so a practice run
|
||||
// needs no retune, using a library filter that only ever looked at the guitar
|
||||
// tuning. Those playlists still hold songs he can't play without stopping. The
|
||||
// check flags them; it must never quietly edit the playlist, and — the part
|
||||
// that decides whether he trusts it — it must not call a song "wrong tuning"
|
||||
// when it simply couldn't work the song out.
|
||||
//
|
||||
// The real functions are lifted out of playlists.js and run in a vm (the module
|
||||
// is a browser IIFE with no export surface, and there is no jsdom here). No
|
||||
// re-implementation: if the source changes, these tests run the changed code.
|
||||
|
||||
'use strict';
|
||||
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const vm = require('node:vm');
|
||||
|
||||
const PL_JS = path.join(__dirname, '..', '..', 'static', 'v3', 'playlists.js');
|
||||
const TUNING_JS = path.join(__dirname, '..', '..', 'static', 'js', 'tuning-display.js');
|
||||
const TUNER_JS = path.join(__dirname, '..', '..', 'plugins', 'tuner', 'screen.js');
|
||||
|
||||
const PL_SRC = fs.readFileSync(PL_JS, 'utf8');
|
||||
|
||||
function extractBlock(src, startMarker) {
|
||||
const start = src.indexOf(startMarker);
|
||||
if (start === -1) throw new Error(`extractBlock: '${startMarker}' not found`);
|
||||
const openBrace = src.indexOf('{', start);
|
||||
let depth = 1;
|
||||
let i = openBrace + 1;
|
||||
while (i < src.length && depth > 0) {
|
||||
const ch = src[i];
|
||||
if (ch === '{') depth++;
|
||||
else if (ch === '}') depth--;
|
||||
i++;
|
||||
}
|
||||
if (depth !== 0) throw new Error(`extractBlock: unbalanced braces after '${startMarker}'`);
|
||||
return src.slice(start, i);
|
||||
}
|
||||
|
||||
// The REAL offset parser the checker calls through window.parseRawTuningOffsets.
|
||||
function loadParseRawTuningOffsets() {
|
||||
const body = fs.readFileSync(TUNING_JS, 'utf8').replace(/^export /gm, '');
|
||||
const sandbox = { window: { feedBack: {} }, exports: {} };
|
||||
vm.createContext(sandbox);
|
||||
vm.runInContext(body + '\nexports.parseRawTuningOffsets = parseRawTuningOffsets;', sandbox);
|
||||
return sandbox.exports.parseRawTuningOffsets;
|
||||
}
|
||||
|
||||
// Build a sandbox holding the real checker functions, over a caller-supplied
|
||||
// window (so each test controls the host capabilities and the coverage stub
|
||||
// boundary). `coverage` stands in for the tuner plugin's coverageReport — a
|
||||
// genuinely external collaborator, not the subject under test; the contract
|
||||
// test at the bottom pins its report shape so these fixtures can't drift.
|
||||
function loadChecker(opts) {
|
||||
opts = opts || {};
|
||||
const calls = [];
|
||||
const window = {
|
||||
parseRawTuningOffsets: loadParseRawTuningOffsets(),
|
||||
feedBack: opts.noWorkingTuning ? {} : { workingTuning: { get: () => ({ instrument: 'bass' }) } },
|
||||
_tunerAutoOpen: opts.noCoverage ? undefined : {
|
||||
coverageReport: async (info) => {
|
||||
calls.push(info);
|
||||
if (opts.coverage) return opts.coverage(info);
|
||||
throw new Error('no coverage fixture supplied');
|
||||
},
|
||||
},
|
||||
};
|
||||
const sandbox = { window, exports: {} };
|
||||
vm.createContext(sandbox);
|
||||
vm.runInContext(
|
||||
extractBlock(PL_SRC, 'function rowTuningForCheck(') + '\n'
|
||||
+ extractBlock(PL_SRC, 'function tuningStateFromReport(') + '\n'
|
||||
+ extractBlock(PL_SRC, 'async function checkPlaylistTuning(') + '\n'
|
||||
+ extractBlock(PL_SRC, 'function tuningSummaryHtml(') + '\n'
|
||||
+ 'exports.rowTuningForCheck = rowTuningForCheck;\n'
|
||||
+ 'exports.tuningStateFromReport = tuningStateFromReport;\n'
|
||||
+ 'exports.checkPlaylistTuning = checkPlaylistTuning;\n'
|
||||
+ 'exports.tuningSummaryHtml = tuningSummaryHtml;\n',
|
||||
sandbox
|
||||
);
|
||||
return { ...sandbox.exports, calls };
|
||||
}
|
||||
|
||||
// Report shapes exactly as plugins/tuner/screen.js documents and returns them.
|
||||
const REPORT_COVERED = { covered: true, retune: [], reference: false, cantCover: false };
|
||||
const REPORT_RETUNE = { covered: false, retune: [{ from: 'E', to: 'D' }], reference: false, cantCover: false };
|
||||
const REPORT_REFERENCE = { covered: false, retune: [], reference: true, cantCover: false };
|
||||
const REPORT_CANT_COVER = { covered: false, retune: [], reference: false, cantCover: true };
|
||||
// The "I couldn't work it out" report — the tuner's `none` bail-out. Byte-for-byte
|
||||
// a not-covered report with no reason attached.
|
||||
const REPORT_UNKNOWN = { covered: false, retune: [], reference: false, cantCover: false };
|
||||
|
||||
// The checker runs inside the vm, so the arrays it returns belong to another
|
||||
// realm and would fail deepStrictEqual's prototype check. Copy into host arrays.
|
||||
const plain = (a) => Array.from(a);
|
||||
|
||||
const song = (over) => Object.assign(
|
||||
{ filename: 'a.sloppak', title: 'A', tuning_name: 'E Standard', tuning_offsets: '0 0 0 0 0 0', bass_only: false },
|
||||
over
|
||||
);
|
||||
|
||||
// ── The unknown-vs-mismatch distinction ─────────────────────────────────────
|
||||
|
||||
test('a covered report is a match', () => {
|
||||
const { tuningStateFromReport } = loadChecker();
|
||||
assert.equal(tuningStateFromReport(REPORT_COVERED), 'match');
|
||||
});
|
||||
|
||||
test('a not-covered report WITH a reason is a mismatch', () => {
|
||||
const { tuningStateFromReport } = loadChecker();
|
||||
assert.equal(tuningStateFromReport(REPORT_RETUNE), 'mismatch');
|
||||
assert.equal(tuningStateFromReport(REPORT_REFERENCE), 'mismatch');
|
||||
assert.equal(tuningStateFromReport(REPORT_CANT_COVER), 'mismatch');
|
||||
});
|
||||
|
||||
test('a not-covered report with NO reason is unknown, not a mismatch', () => {
|
||||
// This is the whole trust argument. The tuner returns this identical shape
|
||||
// when settings/tuner data are missing. Treating it as "wrong tuning" (which
|
||||
// the library grid's chip decorator does) would put a false ⚠ on songs that
|
||||
// are perfectly playable, on a playlist the user curated by hand.
|
||||
const { tuningStateFromReport } = loadChecker();
|
||||
assert.equal(tuningStateFromReport(REPORT_UNKNOWN), 'unknown');
|
||||
});
|
||||
|
||||
test('a null/absent report is unknown', () => {
|
||||
const { tuningStateFromReport } = loadChecker();
|
||||
assert.equal(tuningStateFromReport(null), 'unknown');
|
||||
assert.equal(tuningStateFromReport(undefined), 'unknown');
|
||||
});
|
||||
|
||||
// ── Round-tripping a whole playlist ─────────────────────────────────────────
|
||||
|
||||
test('each song is scored and reported in playlist order', async () => {
|
||||
const byFile = {
|
||||
'match.sloppak': REPORT_COVERED,
|
||||
'bad.sloppak': REPORT_RETUNE,
|
||||
'huh.sloppak': REPORT_UNKNOWN,
|
||||
};
|
||||
const songs = [
|
||||
song({ filename: 'match.sloppak', title: 'Match' }),
|
||||
song({ filename: 'bad.sloppak', title: 'Bad', tuning_offsets: '-2 -2 -2 -2 -2 -2' }),
|
||||
song({ filename: 'huh.sloppak', title: 'Huh', tuning_offsets: '-1 0 0 0 0 0' }),
|
||||
];
|
||||
// Resolve the fixture from the offsets the checker actually passed, so the
|
||||
// mapping can't silently drift out of playlist order.
|
||||
const byOffsets = new Map(songs.map((s) => [s.tuning_offsets.replace(/\s+/g, ','), byFile[s.filename]]));
|
||||
const checker = loadChecker({ coverage: async (info) => byOffsets.get(info.tuning.join(',')) });
|
||||
const out = await checker.checkPlaylistTuning(songs);
|
||||
assert.deepEqual(plain(out.map((r) => r.state)), ['match', 'mismatch', 'unknown']);
|
||||
assert.deepEqual(plain(out.map((r) => r.song.filename)), songs.map((s) => s.filename));
|
||||
});
|
||||
|
||||
test('a song with no usable tuning data is unknown WITHOUT consulting coverage', async () => {
|
||||
// Adversarial payloads: empty, whitespace, a non-numeric name with no
|
||||
// offsets, and a garbage offsets string. None of these can be scored, and
|
||||
// asking coverage about them would invite a bogus not-covered → false ⚠.
|
||||
const checker = loadChecker({ coverage: async () => REPORT_RETUNE });
|
||||
const out = await checker.checkPlaylistTuning([
|
||||
song({ filename: 'a', tuning_offsets: '', tuning_name: '' }),
|
||||
song({ filename: 'b', tuning_offsets: ' ', tuning_name: ' ' }),
|
||||
song({ filename: 'c', tuning_offsets: '', tuning_name: 'E Standard' }),
|
||||
song({ filename: 'd', tuning_offsets: 'not offsets', tuning_name: 'x' }),
|
||||
song({ filename: 'e', tuning_offsets: null, tuning_name: null }),
|
||||
]);
|
||||
assert.deepEqual(plain(out.map((r) => r.state)), ['unknown', 'unknown', 'unknown', 'unknown', 'unknown']);
|
||||
assert.equal(checker.calls.length, 0, 'coverage must not be asked about unscoreable rows');
|
||||
});
|
||||
|
||||
test('a coverage call that throws degrades to unknown, not mismatch', async () => {
|
||||
const checker = loadChecker({ coverage: async () => { throw new Error('tuner exploded'); } });
|
||||
const out = await checker.checkPlaylistTuning([song({})]);
|
||||
assert.deepEqual(plain(out.map((r) => r.state)), ['unknown']);
|
||||
});
|
||||
|
||||
test('a bass-only chart is scored against bass base pitches', async () => {
|
||||
// Otherwise a 4-string bass tuning read as guitar can false-match — the
|
||||
// cross-instrument confusion this whole feature exists to undo.
|
||||
const checker = loadChecker({ coverage: async () => REPORT_COVERED });
|
||||
await checker.checkPlaylistTuning([
|
||||
song({ filename: 'bass', tuning_offsets: '0 0 0 0', bass_only: true }),
|
||||
song({ filename: 'gtr', tuning_offsets: '0 0 0 0 0 0', bass_only: false }),
|
||||
]);
|
||||
assert.deepEqual(checker.calls.map((c) => c.arrangement), ['Bass', 'Lead']);
|
||||
assert.deepEqual(checker.calls.map((c) => c.stringCount), [4, 6]);
|
||||
});
|
||||
|
||||
test('the check stays silent when the host exposes no tuning perspective', async () => {
|
||||
// No working-tuning capability, or no tuner coverage → null, and the caller
|
||||
// renders the playlist exactly as before. Guessing "guitar" here would
|
||||
// reproduce the original bug in a new place.
|
||||
for (const opts of [{ noWorkingTuning: true }, { noCoverage: true }]) {
|
||||
const checker = loadChecker(Object.assign({ coverage: async () => REPORT_COVERED }, opts));
|
||||
assert.equal(await checker.checkPlaylistTuning([song({})]), null);
|
||||
}
|
||||
});
|
||||
|
||||
test('an empty playlist yields an empty result, not a crash', async () => {
|
||||
const checker = loadChecker({ coverage: async () => REPORT_COVERED });
|
||||
assert.deepEqual(plain(await checker.checkPlaylistTuning([])), []);
|
||||
assert.deepEqual(plain(await checker.checkPlaylistTuning(null)), []);
|
||||
});
|
||||
|
||||
// ── The summary ─────────────────────────────────────────────────────────────
|
||||
|
||||
test('the summary counts mismatches against the playlist total', () => {
|
||||
const { tuningSummaryHtml } = loadChecker();
|
||||
const results = [
|
||||
{ state: 'mismatch' }, { state: 'mismatch' }, { state: 'mismatch' },
|
||||
...Array(21).fill({ state: 'match' }),
|
||||
];
|
||||
const html = tuningSummaryHtml(results);
|
||||
assert.match(html, /<strong>3<\/strong> of 24 songs aren't in your tuning/);
|
||||
});
|
||||
|
||||
test('unknowns are reported separately from mismatches and never counted as them', () => {
|
||||
const { tuningSummaryHtml } = loadChecker();
|
||||
const html = tuningSummaryHtml([{ state: 'mismatch' }, { state: 'unknown' }, { state: 'match' }]);
|
||||
assert.match(html, /<strong>1<\/strong> of 3 songs aren't in your tuning/);
|
||||
assert.match(html, /1 couldn't be checked/);
|
||||
assert.match(html, /left alone/);
|
||||
});
|
||||
|
||||
test('an all-unknown playlist makes no mismatch claim and offers no removal', () => {
|
||||
const { tuningSummaryHtml } = loadChecker();
|
||||
const html = tuningSummaryHtml([{ state: 'unknown' }, { state: 'unknown' }]);
|
||||
assert.doesNotMatch(html, /aren't in your tuning/);
|
||||
assert.doesNotMatch(html, /v3-pl-tune-remove/);
|
||||
assert.match(html, /2 couldn't be checked/);
|
||||
});
|
||||
|
||||
test('a clean playlist offers no filter and no removal button', () => {
|
||||
const { tuningSummaryHtml } = loadChecker();
|
||||
const html = tuningSummaryHtml([{ state: 'match' }, { state: 'match' }]);
|
||||
assert.match(html, /All 2 songs are in your tuning/);
|
||||
assert.doesNotMatch(html, /v3-pl-tune-only/);
|
||||
assert.doesNotMatch(html, /v3-pl-tune-remove/);
|
||||
});
|
||||
|
||||
test('an empty playlist renders no summary at all', () => {
|
||||
const { tuningSummaryHtml } = loadChecker();
|
||||
assert.equal(tuningSummaryHtml([]), '');
|
||||
});
|
||||
|
||||
// ── Read-only / explicit-action guarantees (source-level) ───────────────────
|
||||
|
||||
test('the check itself never mutates the playlist', () => {
|
||||
// checkPlaylistTuning and its helpers must contain no write verbs. The only
|
||||
// DELETE in the module's tuning path is inside the confirmed removal.
|
||||
const fns = ['function rowTuningForCheck(', 'function tuningStateFromReport(',
|
||||
'async function checkPlaylistTuning(', 'function tuningSummaryHtml('];
|
||||
for (const marker of fns) {
|
||||
const body = extractBlock(PL_SRC, marker);
|
||||
assert.doesNotMatch(body, /DELETE|jsend\(|method:/,
|
||||
marker + ' must not mutate the playlist');
|
||||
}
|
||||
});
|
||||
|
||||
test('bulk removal names every song and is confirmed before any DELETE', () => {
|
||||
const body = extractBlock(PL_SRC, 'async function applyTuningCheck(');
|
||||
// The confirm is built from the doomed titles …
|
||||
assert.match(body, /doomed\.map\(\(s\) => '<li>' \+ esc\(s\.title \|\| s\.filename\)/);
|
||||
// … it is awaited, and an early return happens before the delete loop.
|
||||
const confirmAt = body.indexOf('uiConfirm');
|
||||
const bailAt = body.indexOf('if (!ok) return;');
|
||||
const deleteAt = body.indexOf("method: 'DELETE'");
|
||||
assert.ok(confirmAt > -1 && bailAt > confirmAt && deleteAt > bailAt,
|
||||
'DELETE must come after an awaited confirm and its bail-out');
|
||||
// And it says the songs survive in the library — the "reversible-feeling" ask.
|
||||
assert.match(body, /stay in your library/);
|
||||
});
|
||||
|
||||
test('removal targets only mismatches — never unknowns', () => {
|
||||
const body = extractBlock(PL_SRC, 'async function applyTuningCheck(');
|
||||
assert.match(body, /results\.filter\(\(r\) => r\.state === 'mismatch'\)\.map\(\(r\) => r\.song\)/);
|
||||
assert.doesNotMatch(body, /doomed[\s\S]{0,200}'unknown'/);
|
||||
});
|
||||
|
||||
test('unknown is styled distinctly from mismatch', () => {
|
||||
const body = extractBlock(PL_SRC, 'function paintTuningChip(');
|
||||
// Mismatch is amber; unknown is the neutral chip, dimmed — not amber.
|
||||
assert.match(body, /state === 'mismatch' \? 'bg-amber-400'/);
|
||||
assert.match(body, /state === 'unknown'\) chip\.classList\.add\('opacity-60'\)/);
|
||||
// …and both carry a text marker, so the states never rest on colour alone.
|
||||
assert.match(body, /state === 'mismatch' \? ' ⚠' : state === 'unknown' \? ' \?'/);
|
||||
});
|
||||
|
||||
// ── Collaborator contract ───────────────────────────────────────────────────
|
||||
|
||||
test('the tuner coverage report still carries the fields the states are read from', () => {
|
||||
// If the tuner plugin drops `retune`/`reference`/`cantCover`, every mismatch
|
||||
// silently degrades to "unknown" and the feature goes quiet. Pin the shape
|
||||
// the fixtures above rely on.
|
||||
const tuner = fs.readFileSync(TUNER_JS, 'utf8');
|
||||
const body = extractBlock(tuner, 'async function _computeCoverageReport(');
|
||||
for (const field of ['covered', 'retune', 'reference', 'cantCover']) {
|
||||
assert.match(body, new RegExp(field), `coverage report must still carry ${field}`);
|
||||
}
|
||||
assert.match(body, /const none = \{ covered: false, retune: \[\], reference: false, cantCover: false \}/,
|
||||
'the no-data bail-out must stay a reasonless not-covered report — that is what "unknown" detects');
|
||||
});
|
||||
@ -266,3 +266,74 @@ def test_new_playlist_after_manual_reorder_sorts_alphabetically_after_positioned
|
||||
assert client.post("/api/playlists/reorder", json={"order": [b, a]}).status_code == 400
|
||||
assert client.post("/api/playlists/reorder", json={"order": [z, aa, b, a]}).status_code == 200
|
||||
assert _ids(client) == [z, aa, b, a]
|
||||
|
||||
|
||||
# ── Tuning-check payload (per-song data the playlist tuning check scores) ────
|
||||
# A playlist grouped BY TUNING is a run you can practise without retuning, so
|
||||
# the detail view flags rows your instrument can't reach. Scoring needs more
|
||||
# than the tuning NAME: two "Custom Tuning" rows are different tunings, and a
|
||||
# bass-only chart has to be measured against bass base pitches.
|
||||
|
||||
def test_playlist_songs_carry_tuning_offsets_for_the_check(client, server):
|
||||
db = server.meta_db
|
||||
db.put("drop.archive", 0, 0, {"title": "Drop", "tuning_name": "Drop D",
|
||||
"tuning_offsets": "-2 0 0 0 0 0"})
|
||||
pid = client.post("/api/playlists", json={"name": "T"}).json()["id"]
|
||||
client.post(f"/api/playlists/{pid}/songs", json={"filename": "drop.archive"})
|
||||
song = client.get(f"/api/playlists/{pid}").json()["songs"][0]
|
||||
assert song["tuning_offsets"] == "-2 0 0 0 0 0"
|
||||
assert song["tuning_name"] == "Drop D"
|
||||
|
||||
|
||||
def test_playlist_songs_flag_bass_only_charts(client, server):
|
||||
# Every arrangement a bass part → bass_only, so coverage scores the row
|
||||
# against bass strings. A chart that ALSO has a guitar part must not be
|
||||
# flagged, or a guitarist's row gets measured on the wrong instrument.
|
||||
db = server.meta_db
|
||||
db.put("bassonly.archive", 0, 0, {"title": "Bass Only", "arrangements": [
|
||||
{"name": "Bass"}, {"name": "Alt. Bass"}]})
|
||||
db.put("mixed.archive", 0, 0, {"title": "Mixed", "arrangements": [
|
||||
{"name": "Lead"}, {"name": "Bass"}]})
|
||||
db.put("noarr.archive", 0, 0, {"title": "No Arrangements"})
|
||||
pid = client.post("/api/playlists", json={"name": "B"}).json()["id"]
|
||||
for fn in ("bassonly.archive", "mixed.archive", "noarr.archive"):
|
||||
client.post(f"/api/playlists/{pid}/songs", json={"filename": fn})
|
||||
got = {s["filename"]: s["bass_only"] for s in client.get(f"/api/playlists/{pid}").json()["songs"]}
|
||||
assert got == {"bassonly.archive": True, "mixed.archive": False, "noarr.archive": False}
|
||||
|
||||
|
||||
def test_bass_only_flag_survives_adversarial_arrangement_data(client, server):
|
||||
# Corrupt/odd `arrangements` must not 500 the playlist, and must not claim
|
||||
# bass — an unscoreable row is left for the client to report as "unknown".
|
||||
db = server.meta_db
|
||||
cases = {
|
||||
"empty.archive": [],
|
||||
"unnamed.archive": [{"name": ""}],
|
||||
"nullname.archive": [{"name": None}],
|
||||
"substring.archive": [{"name": "Bassoon"}], # not a bass part
|
||||
"cased.archive": [{"name": "BASS"}], # is one
|
||||
}
|
||||
for fn, arrs in cases.items():
|
||||
db.put(fn, 0, 0, {"title": fn, "arrangements": arrs})
|
||||
pid = client.post("/api/playlists", json={"name": "Adv"}).json()["id"]
|
||||
for fn in cases:
|
||||
client.post(f"/api/playlists/{pid}/songs", json={"filename": fn})
|
||||
r = client.get(f"/api/playlists/{pid}")
|
||||
assert r.status_code == 200
|
||||
got = {s["filename"]: s["bass_only"] for s in r.json()["songs"]}
|
||||
assert got == {"empty.archive": False, "unnamed.archive": False,
|
||||
"nullname.archive": False, "substring.archive": False,
|
||||
"cased.archive": True}
|
||||
|
||||
|
||||
def test_playlist_song_with_no_tuning_data_reports_empty_not_missing(client, server):
|
||||
# The key must always be present: the client distinguishes "no tuning data"
|
||||
# (unknown — say nothing) from "wrong tuning" (flag it), and a missing key
|
||||
# would make every row unscoreable by accident rather than by fact.
|
||||
db = server.meta_db
|
||||
db.put("bare.archive", 0, 0, {"title": "Bare"})
|
||||
pid = client.post("/api/playlists", json={"name": "Bare"}).json()["id"]
|
||||
client.post(f"/api/playlists/{pid}/songs", json={"filename": "bare.archive"})
|
||||
song = client.get(f"/api/playlists/{pid}").json()["songs"][0]
|
||||
assert song["tuning_offsets"] == ""
|
||||
assert song["bass_only"] is False
|
||||
|
||||
Loading…
Reference in New Issue
Block a user