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

[byrongamatos]

Step 2 of the core-frontend ES-module migration (R3a), on top of #871 and #872. The load-bearing PR — everything the flip needs, landed on its own so it can bake.

What and why

static/app.js is a classic script, so each of its 385 top-level function foo() declarations 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 the file would silently vanish.

This is provably a no-op today. All 66 names are top-level function declarations, so while app.js is still classic, Object.assign(window, {…}) merely re-assigns what window already has. That's exactly what makes it safe to ship alone, ahead of the flip that depends on it.

The consumer set is wider than index.html

The obvious source is the shell's inline on*= handlers. It is not the only one:

Source	Why it's easy to miss
static/v3/index.html inline handlers	the obvious one
handlers app.js builds in template literals	goFavPage, updatePlugin, hideScanBanner, … resolve against window at click time, but live in a JS string — scanning the HTML never finds them
static/v3/*.js (34 files)	showScreen alone has 17 consumers
capabilities/visualization.js:301	reads window.setViz behind a typeof guard — losing it degrades in silence, no error
feedback-desktop + the plugin repos	they live in other repos; no core test covers them

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

The four names no static tool can see

app.js:2156-2157 picks the handler name at runtime:

const letterFn = favoritesOnly ? 'filterFavTreeLetter' : 'filterTreeLetter';
const pageFn   = favoritesOnly ? 'goFavTreePage'       : 'goTreePage';

then interpolates it: `<button onclick="${letterFn}('A')">`.

The names exist only inside string literals. ESLint, no-undef, jsscan, 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. They're hardcoded in the test with a ponytail: comment saying why.

The guard

New tests/js/window_contract.test.js scrapes on*= handlers from both the shell and app.js's template literals, and pins the 4 runtime-composed names by hand.

Verified to bite — a guard that passes both ways guards nothing:

removed from the contract	result
showScreen	✗ inline handlers that would break under type="module"
goTreePage	✗ window.goTreePage is required by the runtime-composed A–Z rail / pagination handlers
setViz	✗ window.setViz is read by another file

Verification

• In-browser: all 66 names resolve on window; the tree view renders 28 A–Z rail buttons carrying their real onclick sources (filterTreeLetter('A'), …) and 8/8 execute with no ReferenceError.
• pytest 2396 passed · test:js 1032/1032 (4 new) · ESLint 0 errors · tailwind-fresh clean.
• Codex preflight 0 — it wrote its own independent scan of both the HTML and app.js's template literals and found zero unexposed handlers.

Not done here

The 21 desktop-only internals (_librarySongId, renderTreeInto, _trapFocusInModal, …) are exposed verbatim — they're an accidental API surface worth narrowing, but that's a behaviour change and doesn't belong in a move-only PR. Worth a follow-up issue.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Preserved existing app interactions and controls when the application is loaded using modern module behavior.
  • Improved compatibility for inline actions, navigation controls, pagination, and other dynamically generated interface actions.
  • Added safeguards to ensure all required interactive actions remain available and responsive across the application.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an explicit window export contract for app functions and tests that inline, runtime-composed, and cross-file handler names resolve against that contract.

Changes

Window Contract Exposure

Layer / File(s)	Summary
Global window export contract static/app.js	Adds an Object.assign(window, { ... }) block exposing selected app functions globally.
Window contract validation tests/js/window_contract.test.js	Adds static checks for exported names referenced by HTML handlers, template literals, runtime-composed handlers, and cross-file readers.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: making app.js's window contract explicit with 66 exported names.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/r3-window-contract

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

tests/js/window_contract.test.js (1)

46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Broaden the handler regex in tests/js/window_contract.test.js:46. The current event list misses handlers already present in the scan target, e.g. ondblclick in static/v3/index.html, so the guard can skip inline handlers on uncovered events.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/js/window_contract.test.js` at line 46, Broaden the HANDLER regular
expression to recognize all inline event handler names present in the scan
target, including ondblclick, while preserving the existing handler-name capture
and matching behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/js/window_contract.test.js`:
- Line 46: Broaden the HANDLER regular expression to recognize all inline event
handler names present in the scan target, including ondblclick, while preserving
the existing handler-name capture and matching behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b7a344ef-d848-4afe-bfca-19334a38ab8b

📥 Commits

Reviewing files that changed from the base of the PR and between 4b4c156 and 69002c1.

📒 Files selected for processing (2)

• static/app.js
• tests/js/window_contract.test.js