refactor(app): make app.js's window contract explicit — 66 names (R3a) (#874)

app.js is a classic script, so each of its 385 top-level `function foo()` decls
is implicitly a property of `window`. As an ES module it will not be — module
scope is not global scope — and every name reached from outside this file would
silently vanish. This adds the explicit `window.*` assignments BEFORE the flip.

Provably a NO-OP: all 66 are top-level function declarations, so while app.js is
still a classic script `Object.assign(window, {...})` only re-assigns what
`window` already has. That is what makes it safe to land on its own, ahead of
the flip that needs it.

The consumers are wider than the inline handlers in index.html:
  - inline on*= handlers in static/v3/index.html
  - on*= handlers app.js BUILDS inside template literals (goFavPage,
    updatePlugin, hideScanBanner, ...) — they resolve against window at CLICK
    time, but live in a JS string, so scanning the HTML alone never finds them
  - static/v3/*.js (showScreen alone has 17 consumers), capabilities
  - feedback-desktop and the external plugin repos — easy to miss, they live in
    other repos and no core test covers them
  - capabilities/visualization.js reads window.setViz behind a `typeof` guard,
    so losing it DEGRADES IN SILENCE rather than throwing

Constitution II names window.playSong / window.showScreen / window.feedBack as
the public extension contract, so this is an obligation, not a convenience.

FOUR names are invisible to every static tool. app.js:2156-2157 picks the
handler NAME at runtime —
    const letterFn = favoritesOnly ? 'filterFavTreeLetter' : 'filterTreeLetter';
— and interpolates it into `onclick="${letterFn}('A')"`. The names exist only
inside string literals, so ESLint, no-undef, and any grep for `onclick="fn` all
miss them. They are the library A-Z rail and its pagination: drop one and those
buttons throw at click time and nowhere else.

New tests/js/window_contract.test.js scrapes the HTML's handlers AND app.js's
template-literal handlers, and pins the 4 runtime-composed names by hand.
Verified to BITE: dropping showScreen, goTreePage, or setViz each fails it with
the right message.

On-device: 28 A-Z rail buttons render with their real onclick sources
(filterTreeLetter('A'), ...) and 8/8 execute with no ReferenceError; all 66
names resolve on window in the browser.

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

static/app.js

@@ -12165,3 +12165,52 @@ async function bootstrapPluginsAndUi() {
         })
         .catch(() => {});
 })();
+
+
+// ─── The window contract ────────────────────────────────────────────────────
+// app.js is a classic script today, so every top-level `function foo()` here is
+// implicitly a property of `window`. The R3a migration turns this file into an
+// ES module, where that stops being true — module scope is not global scope, and
+// each of these names would silently vanish from `window`.
+//
+// Everything below is reached by NAME from outside this file, so each one is
+// made explicit BEFORE the flip. While app.js is still classic this whole block
+// is a no-op (it just re-assigns what is already there), which is exactly what
+// makes it safe to land on its own.
+//
+// The consumers are: inline on*= handlers in static/v3/index.html; on*= handlers
+// this file builds inside template literals; static/v3/*.js; the capabilities;
+// bundled plugins; and — easy to forget, since they live in other repos —
+// feedback-desktop and the external plugins. Constitution II names
+// `window.playSong` / `window.showScreen` / `window.feedBack` as the public
+// extension contract.
+//
+// Guarded by tests/js/window_contract.test.js. Add a name here the moment
+// anything outside app.js calls it.
+Object.assign(window, {
+    _confirmDialog, _getArrangementNamingMode, _libraryLocalFilename, _librarySongArtUrl,
+    _librarySongId, _onHeaderClick, _onNamingModeChange, _trapFocusInModal,
+    changeArrangement, checkPluginUpdates, clearLibFilters, clearLoop,
+    deleteSelectedLoop, exportDiagnostics, exportSettings, filterFavorites,
+    filterLibrary, fullRescanLibrary, goFavPage, handleSliderInput,
+    hideScanBanner, importSettings, loadPlugins, loadSavedLoop,
+    loadSettings, onSectionPracticeModeChange, openEditModal, persistSetting,
+    pickDlcFolder, pinCurrentArrangementDefault, playSong, previewDiagnostics,
+    previewEditArt, renderGridCards, renderTreeInto, rescanLibrary,
+    retuneSong, saveCurrentLoop, saveSettings, seekBy,
+    setAvOffsetMs, setFavView, setInstrumentPathway, setLibView,
+    setLibraryProvider, setLoopEnd, setLoopStart, setMastery,
+    setSpeed, setViz, showScreen, sortFavorites,
+    sortLibrary, syncLibrarySong, toggleAllArtists, toggleAllFavoriteArtists,
+    toggleLibFilters, togglePlay, toggleSectionPracticePopover, uiPrompt,
+    updatePlugin, uploadSongs,
+
+    // These four are invisible to every static scan. app.js:2156-2157 picks the
+    // handler NAME at runtime —
+    //     const letterFn = favoritesOnly ? 'filterFavTreeLetter' : 'filterTreeLetter';
+    // — and interpolates it: `onclick="${letterFn}('A')"`. So the names never
+    // appear as identifiers anywhere, and ESLint / no-undef / a grep for
+    // `onclick="fn` all miss them. They are the library A-Z rail and its
+    // pagination; drop one and those buttons throw at click time, nowhere else.
+    filterFavTreeLetter, filterTreeLetter, goFavTreePage, goTreePage,
+});

tests/js/window_contract.test.js

@@ -0,0 +1,95 @@
+// Guards app.js's `window` contract ahead of the R3a ES-module flip.
+//
+// app.js is a classic script, so every top-level `function foo()` is implicitly
+// a property of `window`. As an ES module it will not be — module scope is not
+// global scope. Any name reached from OUTSIDE app.js must therefore be an
+// explicit `window.foo = …` before the flip, or it vanishes silently.
+//
+// "Silently" is the whole problem. A missing inline handler is a ReferenceError
+// only when someone clicks the button; a `typeof window.setViz !== 'function'`
+// guard (capabilities/visualization.js) just degrades and says nothing. Neither
+// shows up in a test run, so this file is the thing standing between a dropped
+// name and a dead button in production.
+
+const { test } = require('node:test');
+const assert = require('node:assert/strict');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const ROOT = path.join(__dirname, '..', '..');
+const APP_JS = fs.readFileSync(path.join(ROOT, 'static', 'app.js'), 'utf8');
+const V3_HTML = fs.readFileSync(path.join(ROOT, 'static', 'v3', 'index.html'), 'utf8');
+
+// Every name app.js publishes: the scattered `window.foo = …` assignments plus
+// the consolidated `Object.assign(window, { … })` contract block at the bottom.
+function exposedNames() {
+    const names = new Set(
+        [...APP_JS.matchAll(/^window\.([A-Za-z_$][\w$]*)\s*=/gm)].map((m) => m[1]),
+    );
+    const block = APP_JS.match(/Object\.assign\(window, \{([\s\S]*?)\n\}\);/);
+    assert.ok(block, 'the Object.assign(window, …) contract block is missing from app.js');
+    // Strip the comments first — the prose inside them is full of words that
+    // would otherwise scrape as identifiers.
+    const body = block[1].replace(/\/\/[^\n]*/g, '');
+    for (const m of body.matchAll(/([A-Za-z_$][\w$]*)\s*(?=,|$)/gm)) names.add(m[1]);
+    return names;
+}
+
+// app.js's own top-level `function foo()` declarations — the names that stop
+// being global under `type="module"`.
+function topLevelFunctions() {
+    return new Set(
+        [...APP_JS.matchAll(/^(?:async\s+)?function\s+([A-Za-z_$][\w$]*)/gm)].map((m) => m[1]),
+    );
+}
+
+const HANDLER = /on(?:click|change|input|submit|keyup|keydown|mousedown|error|focus|blur)\s*=\s*"([A-Za-z_$][\w$]*)/g;
+
+test('every inline on*= handler in the v3 shell is on window', () => {
+    const exposed = exposedNames();
+    const owned = topLevelFunctions();
+    const missing = [...V3_HTML.matchAll(HANDLER)]
+        .map((m) => m[1])
+        .filter((n) => owned.has(n) && !exposed.has(n));
+    assert.deepEqual([...new Set(missing)], [], 'inline handlers that would break under type="module"');
+});
+
+test('every on*= handler app.js builds in a template literal is on window', () => {
+    // e.g. `<button onclick="goFavPage(${p})">` — these resolve against window at
+    // CLICK time, exactly like the ones written into the HTML, but they live in a
+    // JS string so scanning index.html alone never finds them.
+    const exposed = exposedNames();
+    const owned = topLevelFunctions();
+    const missing = [...APP_JS.matchAll(HANDLER)]
+        .map((m) => m[1])
+        .filter((n) => owned.has(n) && !exposed.has(n));
+    assert.deepEqual([...new Set(missing)], [], 'generated handlers that would break under type="module"');
+});
+
+test('the runtime-composed handler names are on window', () => {
+    // app.js:2156-2157 chooses the handler NAME at runtime:
+    //     const letterFn = favoritesOnly ? 'filterFavTreeLetter' : 'filterTreeLetter';
+    //     const pageFn   = favoritesOnly ? 'goFavTreePage'       : 'goTreePage';
+    // then interpolates it: `onclick="${letterFn}('A')"`.
+    //
+    // ponytail: hardcoded on purpose. These names exist only inside string
+    // literals, so the two scans above cannot see them, and neither can ESLint,
+    // no-undef, or a grep for `onclick="fn`. They are the library A–Z rail and
+    // its pagination — drop one and those buttons throw on click and nowhere
+    // else. If that ternary ever gains a branch, add the new name here too.
+    const exposed = exposedNames();
+    for (const name of ['filterTreeLetter', 'filterFavTreeLetter', 'goTreePage', 'goFavTreePage']) {
+        assert.ok(exposed.has(name), `window.${name} is required by the runtime-composed A–Z rail / pagination handlers`);
+    }
+});
+
+test('cross-file window.* readers still resolve', () => {
+    // Names other core scripts read off window. capabilities/visualization.js is
+    // the cautionary one: it reads window.setViz behind a `typeof` guard, so
+    // losing it degrades the visualization capability in SILENCE rather than
+    // throwing.
+    const exposed = exposedNames();
+    for (const name of ['setViz', 'showScreen', 'playSong', 'uiPrompt', '_confirmDialog', 'loadPlugins']) {
+        assert.ok(exposed.has(name), `window.${name} is read by another file`);
+    }
+});