got-feedBack/feedBack
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack.git synced 2026-07-21 20:31:21 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

feat(v3): flag library songs by working-tuning match (working-tuning PR 6) (#668)

Browse Source 
* feat(v3): flag library songs by working-tuning match (working-tuning PR 6)

Each song's tuning chip in the v3 library grid is now coloured by whether your
CURRENT working tuning covers it: green = play it now, amber = needs a retune
(with a matching tooltip). Uses the tuner plugin's coverageReport (async), so it
runs as a post-paint decoration pass — chips render instantly, then colour a tick
later; a token cancels a superseded pass so scrolling stays snappy. Re-flags on
working-tuning-changed (retune / instrument swap / reset), no re-fetch.

Fully feature-detected: without the tuner coverage API + the host workingTuning
state, the chips render exactly as before. v3-only, single file (static/v3/songs.js).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF

* fix(tuner/library): correct bass matching + memoize player tuning (PR #668 review)

Review fixes for working-tuning PR 6 (library tuning-match chips):

- Bass songs were scored against the guitar tuning. The chip passed no arrangement
  to coverageReport, so isBassArrangement fell back to guitar — a 4-string bass
  drop-D read as guitar could FALSE-MATCH a drop-D guitar player (green). songCard
  now flags a bass-only song (every arrangement name matches /\bbass\b/) with
  data-tuning-bass, and decorateTuningChips passes arrangement 'Bass'/'Lead' so
  coverage uses the right base pitches. Mixed guitar+bass songs → guitar (the
  song-level tuning is the guitar one); least-wrong given one tuning per song.

- Per-chip /api/settings fetch storm. coverageReport()→_playerTuning() fetched
  /api/settings once per visible chip per grid paint (~60). _playerTuning is now
  memoized (the player's tuning is song-independent) so all callers share one read;
  invalidated on instrument:changed / working-tuning-changed, with a 3s TTL so a
  settings write that doesn't emit an event still heals. A transient fetch failure
  is NOT cached (next read retries) — else one hiccup would freeze coverage.

Tests: player tuning shared across songs (one fetch); transient-failure retry
(fails without the fix). The prior #680 dedup test updated for the memoized behavior.

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

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
This commit is contained in:
ChrisBeWithYou 2026-07-01 03:51:33 -05:00 committed by GitHub
parent 6aed8510d7
commit df4e17bc99
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 114 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
plugins/tuner/screen.js
View File

@ -189,7 +189,32 @@
return { isBass, sc, key: (isBass ? 'bass' : 'guitar') + '-' + sc };
}
async function _playerTuning() {
// Memoize the player's tuning: it depends only on /api/settings + the working
// tuning, which change on instrument:changed / working-tuning-changed (NOT per song).
// Without this, a consumer that evaluates coverage for many items at once — the
// library's per-song tuning-match chips — would fire one /api/settings fetch PER item
// per paint. Cache the promise so they all share one read; invalidate on the events
// that change the answer, AND expire after a short TTL so a settings write that
// doesn't emit an event (e.g. the input-setup flow) still heals within seconds.
let _playerTuningPromise = null;
let _playerTuningAt = 0;
const _PLAYER_TUNING_TTL_MS = 3000;
function _playerTuning() {
const now = (typeof Date !== 'undefined' && Date.now) ? Date.now() : 0;
if (!_playerTuningPromise || (now - _playerTuningAt) > _PLAYER_TUNING_TTL_MS) {
_playerTuningAt = now;
const p = _computePlayerTuning();
_playerTuningPromise = p;
// Never pin a transient failure: if the read yields null (or rejects), drop
// the cache so the next call retries. A real result stays until invalidation/TTL.
p.then((r) => { if (r == null && _playerTuningPromise === p) _playerTuningPromise = null; },
() => { if (_playerTuningPromise === p) _playerTuningPromise = null; });
}
return _playerTuningPromise;
}
function _invalidatePlayerTuning() { _playerTuningPromise = null; }
async function _computePlayerTuning() {
const u = window._tunerUtils;
if (!u) return null;
// Instrument IDENTITY (which instrument is selected) + the static fallback
@ -408,10 +433,12 @@
// switches instrument. Drop the cached selection so a publish-on-clear can't write
// to the previously-selected instrument's slot; the next coverage read re-resolves
// it. Until then _publishWorkingTuning skips (safe — no mis-slotted write).
window.feedBack.on('instrument:changed', () => { _state._playerSelected = null; _invalidateCoverageCache(); });
window.feedBack.on('instrument:changed', () => {
_state._playerSelected = null; _invalidatePlayerTuning(); _invalidateCoverageCache();
});
// A retune (working tuning published on a tuner clear) changes coverage for the
// current song — drop the cached report so a re-evaluation recomputes it.
window.feedBack.on('working-tuning-changed', _invalidateCoverageCache);
// current song — drop the cached player tuning + report so a re-evaluation recomputes.
window.feedBack.on('working-tuning-changed', () => { _invalidatePlayerTuning(); _invalidateCoverageCache(); });
}
// ── Player sync helpers ───────────────────────────────────────────

59
static/v3/songs.js
View File

@ -604,6 +604,41 @@
'<button data-arr="' + esc(a.index != null ? a.index : '') + '" title="Play ' + esc(a.name) + '" class="text-[10px] px-1.5 py-0.5 rounded bg-gray-800/60 text-fb-textDim hover:bg-fb-primary hover:text-white transition">' + esc(a.name) + '</button>').join('');
}
// ── Tuning-match flags (working-tuning PR 6) ───────────────────────────────
// Colour each song's tuning chip by whether your CURRENT working tuning covers
// it: green = play it now, amber = needs a retune. Uses the tuner plugin's
// coverage check (async) + the host workingTuning state — BOTH feature-detected,
// 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;
function _applyChipMatch(chip, stateName) {
chip.classList.remove('bg-fb-mid', 'bg-emerald-500', 'bg-amber-400');
chip.classList.add(stateName === 'match' ? 'bg-emerald-500'
: stateName === 'retune' ? 'bg-amber-400' : 'bg-fb-mid');
if (!chip.dataset.baseTitle) chip.dataset.baseTitle = chip.getAttribute('title') || '';
chip.setAttribute('title', chip.dataset.baseTitle + (stateName === 'match'
? ' — matches your tuning' : stateName === 'retune' ? ' — needs a retune' : ''));
}
async function decorateTuningChips(grid) {
if (!grid) return;
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; // feature-detect → no flags
const token = ++_tuningDecorToken;
const chips = grid.querySelectorAll('[data-tuning-chip][data-tuning-offsets]');
for (const chip of chips) {
const offs = chip.getAttribute('data-tuning-offsets').split(',').map(Number);
if (!offs.length || offs.some((n) => !isFinite(n))) continue;
// Pass the instrument so coverage uses the right base pitches (bass vs guitar).
const arrangement = chip.dataset.tuningBass === '1' ? 'Bass' : 'Lead';
let rep = null;
try { rep = await cov({ tuning: offs, stringCount: offs.length, arrangement: arrangement }); } catch (_) { rep = null; }
if (token !== _tuningDecorToken) return; // superseded by a re-paint / tuning change
if (rep) _applyChipMatch(chip, rep.covered ? 'match' : 'retune');
}
}
function songCard(song) {
const fav = song.favorite;
const key = cardKey(song);
@ -626,11 +661,22 @@
? ('Custom Tuning: ' + targetNotes)
: tuningLabel;
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.
// Also flag a bass-only song (every arrangement is a bass part) so coverage
// 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 = song.arrangements || [];
const chipIsBass = 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"' : '') : '';
if (targetNotes) {
tuning = '<span class="' + pos + ' bg-fb-mid text-black text-[9px] font-bold px-1.5 py-0.5 rounded-sm leading-tight max-w-[5.5rem] text-center" title="' + esc(badgeTitle) + '">'
tuning = '<span class="' + pos + ' bg-fb-mid text-black text-[9px] font-bold px-1.5 py-0.5 rounded-sm leading-tight max-w-[5.5rem] text-center"' + matchAttr + ' title="' + esc(badgeTitle) + '">'
+ esc('Custom Tuning') + '<br><span class="font-semibold tracking-wide">' + esc(targetNotes) + '</span></span>';
} else {
tuning = '<span class="' + pos + ' bg-fb-mid text-black text-[10px] font-bold px-1.5 py-0.5 rounded-sm" title="' + esc(badgeTitle) + '">' + esc(tuningLabel) + '</span>';
tuning = '<span class="' + pos + ' bg-fb-mid text-black text-[10px] font-bold px-1.5 py-0.5 rounded-sm"' + matchAttr + ' title="' + esc(badgeTitle) + '">' + esc(tuningLabel) + '</span>';
}
}
// Display-only (pointer-events-none) so a click falls through to the
@ -1032,6 +1078,7 @@
grid.style.top = (firstRow * rowH) + 'px';
grid.innerHTML = _renderCardsRange(start, end);
wireCards(grid);
decorateTuningChips(grid); // colour tuning chips by working-tuning match (async, feature-detected)
state.winRange = { start, end };
state.renderedSelectMode = state.selectMode;
if (sm && typeof sm.emit === 'function') {
@ -1858,5 +1905,13 @@
if (active && active.id === 'v3-songs') { _libraryDirty = false; reload(); }
else _libraryDirty = true;
});
// Your live tuning changed (retune / instrument swap / reset) → re-colour the
// 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', () => {
if (typeof songsActive === 'function' && !songsActive()) return;
if (state.view !== 'grid') return;
decorateTuningChips(_gridEl());
});
}
})();

31
tests/js/tuner_auto_open.test.js
View File

@ -591,9 +591,11 @@ test('a configured standard-guitar player still covers a standard song', async (
const covered = await sandbox.window._tunerAutoOpen.coveredByPlayerInstrument(E_STANDARD);
assert.equal(covered, true, 'a known standard guitar covers a standard song (no regression)');
});
// ── #657 fix (#680): coverage is deduped — the auto-open gate and the badge cue both
// call coverageReport() on the same song:ready; they must share ONE /api/settings fetch.
test('coverage reports for the same song share one settings fetch, and a new song refetches', async () => {
// ── #657 fix (#680) + #668 fix: coverage is deduped, and the player tuning is memoized
// across songs (it depends on the selected instrument, not the song). Many coverage
// calls — the auto-open gate, the badge cue, AND the library's per-song tuning-match
// chips — share ONE /api/settings fetch until the instrument / working tuning changes.
test('coverage reports share one /api/settings fetch across songs (player tuning memoized)', async () => {
const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } });
let settingsFetches = 0;
const origFetch = sandbox.window.fetch;
@ -605,8 +607,27 @@ test('coverage reports for the same song share one settings fetch, and a new son
const [a, b] = await Promise.all([api.coverageReport(DROP_D), api.coverageReport(DROP_D)]);
assert.equal(settingsFetches, 1, 'concurrent reports for the same song share one fetch');
assert.deepEqual(a, b);
// A new song invalidates the cache → a fresh fetch.
// A different song re-evaluates coverage but reuses the memoized player tuning — the
// player didn't retune or switch instruments, so no second /api/settings read.
api.onSongLoading();
await api.coverageReport(E_STANDARD);
assert.equal(settingsFetches, 2, 'a new song refetches');
assert.equal(settingsFetches, 1, 'a different song reuses the memoized player tuning — no refetch');
});
// ── #668 fix: a transient /api/settings failure must NOT be pinned by the player-tuning
// memo — the next read retries (else one hiccup freezes coverage as "unknown" for good).
test('a transient /api/settings failure is not cached — the next coverage read retries', async () => {
const sandbox = createTunerSandbox({ player: { instrument: 'guitar', string_count: 6, tuning: 'Standard' } });
let failNext = true;
const origFetch = sandbox.window.fetch;
sandbox.window.fetch = (url) => {
if (String(url).includes('/api/settings') && failNext) { failNext = false; return Promise.reject(new Error('boom')); }
return origFetch(url);
};
const api = sandbox.window._tunerAutoOpen;
const r1 = await api.coverageReport(DROP_D); // settings read failed → conservative "none" report
assert.equal(r1.retune.length, 0, 'a fetch failure yields the empty/unknown report');
api.onSongLoading(); // clear the coverage cache to force a recompute
const r2 = await api.coverageReport(DROP_D); // retry: settings now readable → a real report
assert.equal(r2.retune.length, 1, 'the retry actually computes coverage (Drop-D low string vs standard)');
});