2 Commits

Author	SHA1	Message	Date
ChrisBeWithYou	cc75cb876a	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>	2026-07-19 00:04:30 -05:00
Byron Gamatos	bd830328f0	refactor(app): carve the library out of app.js (R3a) (#896) ... static/js/library.js (1,988) + static/js/library-state.js (29) — bodies VERBATIM. app.js 6,313 -> 4,451. THE BIGGEST SLICE OF THE CARVE: 145 declarations, ~1,900 lines, 30% of what was left. The grid, the artist tree, the A-Z rail, filters, pagination, selection, favourites, the scan banner, and the library-provider plumbing. A LOW module: it imports only leaves (./dom.js, ./format.js, ./library-state.js, ./tuning-display.js — all four import nothing themselves) and needs ZERO host hooks. It calls nothing in app.js. That is not luck; it is why this cluster was picked. Two entry points that WOULD have dragged the playback core in were left behind in app.js: * syncLibrarySong reaches showScreen/playSong * _handleLibArrowNav Enter on a selected row plays the song Both are one hop from the library, and app.js is the root, so it imports from both sides for free. Pulling them in swallows playSong, showScreen and the whole remaining core — I measured it: the closure jumps from 145 declarations to 189. library-state.js holds exactly FIVE fields. An imported binding is read-only, and of the library's outward bindings only these five are genuinely WRITTEN from outside — by showScreen, deleteSongFromModal and syncLibrarySong, none of which can move in. The other 23 are read-only from outside, so they stay plain exports (ES live bindings mean app.js still sees every reassignment). ━━━ THE EXPORT LIST NEARLY SHIPPED A DEAD A-Z RAIL ━━━ 59 exports — and 43 of them CANNOT be found by a call-graph scan. They are referenced only from app.js's TOP-LEVEL statements: the Object.assign(window, {...}) contract and the scattered window.X = X lines, which live outside every function, so a closure walk over declarations never sees them. Among them are the four handler names app.js composes AT RUNTIME into onclick="" strings — filterTreeLetter, filterFavTreeLetter, goTreePage, goFavTreePage — the library A-Z rail and its pagination. No static tool can see those at all. Had I trusted the call-graph, the rail would have died silently on click with nothing failing in CI. ━━━ AND MY OWN SCANNER LIED ━━━ The cycle-risk pass reported "(none)" for this carve. It was wrong, and it could not have been right: a dangling `else if` bound to an inner `if` instead of the outer chain, so its `imported` map was ALWAYS empty and the check reported clean no matter what. A guard that cannot fail is worse than no guard. Fixed, and it then found the real edges — dom.js, format.js, tuning-display.js, library-state.js. All four are leaves, so the carve is genuinely acyclic; I just now know it instead of assuming it. (The AST rewriter had its own trap: `MAP[name]` with an object literal and name === 'constructor' hits Object.prototype.constructor — truthy — and it happily rewrote `constructor(id)` into `L.function Object() { [native code] }(id)`. Every identifier in the file is looked up, so the lookup must not see the prototype chain. It is a Map now.) TESTS. legacy_shim_hits SPLIT (loadLibraryProviders + setLibraryProvider -> the module; syncLibrarySong stayed in app.js). v3_library_refresh now reads app.js AND the module, rather than being re-pinned to whichever file happens to hold the emit this week. VERIFIED. A/B against origin/main in two browsers: the whole window contract, cards render, grid/tree/sort/filter/clear round-trip — and, specifically, the A-Z rail: 28 onclick handlers composed at runtime, identical on both, and a real .click() on a letter works. IDENTICAL on all 33 + 7 probes, no new page errors. pytest 2396, node 1040/1040, host contract 2/2, ESLint 0 (no-cycle clean). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-11 23:30:30 +02:00