refactor(ui): defer every classic script, keep boot() on DOMContentLoaded (R3a)

[byrongamatos]

Step 1 of the core-frontend ES-module migration (R3a), on top of #871. Puts every external <script> in the v3 shell into the deferred queue while keeping each script's boot() firing at DOMContentLoaded exactly as it does today. Behaviourally a no-op — it exists to make the module flips (PR-3, PR-4) safe.

Why

type="module" defers execution to after HTML parse. Classic-defer and module scripts share one "execute after parsing" list and run in document order — but a plain classic script runs during parse, ahead of all of them.

So the moment capabilities.js becomes a module while app.js is still plain, app.js runs first, and its 11 top-level window.feedBack.on(...) calls (app.js:6245-6722) hit a bare {}. _ensureFeedBackEventBus() (capabilities.js:33) — which attaches .on/.emit/.off — hasn't run yet. TypeError, app.js dies mid-parse.

The bus is order-tolerant for properties (_ensureFeedBackEventBus copies keys off a bare {} and replaces it). It is not tolerant of .on() being called before it exists. Deferring everything keeps document order == execution order for the rest of the migration.

The catch — a real ordering change, caught by Codex preflight

22 scripts guard their boot with if (document.readyState === 'loading'). A deferred script runs at readyState 'interactive', so that test is false and the else-branch fires boot() immediately, at the script's position in document order — instead of at DOMContentLoaded, after every script has evaluated.

That matters much more than the one call site Codex named. A scan of the shell found 43 forward references where a script's boot() reads a global a later script defines:

reader	global	defined later by
v3/shell.js	window.v3Onboarding	v3/profile.js
v3/songs.js	window._confirmDialog	v3/settings.js
v3/badges.js	window.displayTuningName	v3/songs.js
v3/playlists.js	window.uiPrompt	v3/image-picker.js
app.js	window.fbNotify	v3/notifications.js
…38 more

Every one resolves today only because all boots happen at DOMContentLoaded. So the guards now treat 'interactive' as not-ready (!== 'complete'), restoring that exactly. Each site carries a one-line note so it doesn't get "fixed" back.

On Codex's specific finding (first-run onboarding silently skipped): it did not reproduce. shell.js's boot() awaits /api/profile, and that yield lets the remaining deferred scripts run first — I traced it on both branches to confirm. But the race it describes is real, the guard fails silently (&& window.v3Onboarding — no error, onboarding just never shows), and the other 42 forward refs have no await protecting them. Fixed at the root rather than at the one site.

Verification

A/B against origin/main on a fresh profile (onboarded: false), 13 probes — onboarding overlay, v3Onboarding / v3Songs / v3Profile / fbNotify / v3Badges / uiPrompt / showScreen, the bus, capabilities.version, createHighway, loaded plugin scripts, mounted plugin screens:

IDENTICAL on every probe. Zero console/page errors on both.

• pytest 2396 passed, 4 skipped · node test:js 1028/1028 · ESLint 0 errors · tailwind-fresh clean · Codex preflight 0.
• R0 plugin rails intact under defer: 14 plugin scripts loaded, 5 screens mounted, the 3 migrated plugins still injected as <script type="module">.

New guard: test_every_external_script_defers_so_document_order_is_execution_order fails if any external tag is plain classic. Verified it actually bites — reverting a single tag fails it with external scripts that would jump the deferred queue: ['<script src="/static/app.js">']. (A guard that passes both ways guards nothing.)

Note on the 4 inline <script> blocks

Left parse-time on purpose. :55 (ss-follower) and :69 (interface-scale pre-paint, explicitly "before any stylesheet paints") must run before paint; the other two are self-contained. None reads a global from a deferred script — checked.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved startup timing across the app to ensure interface components initialize only after the page is ready.
  • Reduced potential loading-order issues that could affect prompts, dashboards, audio controls, settings, playback, and other interactive features.

• Tests

  • Added coverage to verify external scripts load and execute in the correct order.

[coderabbitai]

📝 Walkthrough

Walkthrough

External v3 scripts now use defer, while application and feature initialization waits for DOMContentLoaded unless the document is already complete. Tests verify deferred external script loading and document-order assumptions.

Changes

Deferred initialization ordering

Layer / File(s)	Summary
Deferred script loading contract static/v3/index.html, tests/test_plugin_runtime_idempotence.py	V3 external scripts use defer, and tests verify that external scripts are deferred and retain required document order.
Shared readiness gates static/app.js, static/audio-mixer.js, static/capabilities/interface-scale.js	Initialization now waits for DOMContentLoaded in every non-complete document state.
V3 bootstrap readiness gates static/v3/*.js	V3 shell, feature, player, and interface startup paths defer until DOMContentLoaded unless the document is complete.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HTMLParser
  participant DeferredScripts
  participant DOMContentLoaded
  participant BootstrapModules
  HTMLParser->>DeferredScripts: Execute external scripts after parsing
  DeferredScripts->>BootstrapModules: Register DOMContentLoaded handlers
  DOMContentLoaded->>BootstrapModules: Invoke boot/init/refresh handlers
  BootstrapModules->>BootstrapModules: Initialize application features

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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: deferring classic scripts and keeping boot() on DOMContentLoaded.
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-defer-classic-scripts

---

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

[coderabbitai]

🧹 Nitpick comments (1)

tests/test_plugin_runtime_idempotence.py (1)

101-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Substring checks can produce false negatives.

'defer' not in tag, 'async' not in tag, and 'type="module"' not in tag match anywhere in the raw tag text, not just attribute names. A future tag with an unrelated attribute containing those substrings (e.g. data-async-load="x") would be misclassified as deferred/async and silently skip the check this test exists to enforce — precisely the regression this test is meant to catch. A word-boundary/attribute-aware regex would be more robust.

♻️ Suggested tightening

     plain = [
         tag for tag in re.findall(r'<script\b[^>]*\bsrc=[^>]*>', source)
-        if 'defer' not in tag and 'async' not in tag and 'type="module"' not in tag
+        if not re.search(r'\bdefer\b', tag)
+        and not re.search(r'\basync\b', tag)
+        and 'type="module"' not in tag
     ]

🤖 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/test_plugin_runtime_idempotence.py` around lines 101 - 105, Update the
script-tag filtering assertion to detect defer, async, and type="module" as
actual attribute names/values rather than arbitrary substrings in raw tag text.
Use attribute-aware or word-boundary matching so unrelated attributes such as
data-async-load do not bypass the plain-script check, while preserving the
existing external-script detection.

🤖 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/test_plugin_runtime_idempotence.py`:
- Around line 101-105: Update the script-tag filtering assertion to detect
defer, async, and type="module" as actual attribute names/values rather than
arbitrary substrings in raw tag text. Use attribute-aware or word-boundary
matching so unrelated attributes such as data-async-load do not bypass the
plain-script check, while preserving the existing external-script detection.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 12e6a43c-b1a9-4eba-b457-f255f330dee3

📥 Commits

Reviewing files that changed from the base of the PR and between 9d0bf95 and f69d1f7.

📒 Files selected for processing (24)

• static/app.js
• static/audio-mixer.js
• static/capabilities/interface-scale.js
• static/v3/badges.js
• static/v3/dashboard.js
• static/v3/feedbarcade.js
• static/v3/index.html
• static/v3/interface-size-nudge.js
• static/v3/interface-size-ui.js
• static/v3/live-guitar-tone-source.js
• static/v3/live-performance-hud.js
• static/v3/match-review.js
• static/v3/player-chrome.js
• static/v3/playlists.js
• static/v3/plugins-page.js
• static/v3/profile.js
• static/v3/progress.js
• static/v3/progression-core.js
• static/v3/settings.js
• static/v3/shell.js
• static/v3/shop.js
• static/v3/venue-mood-fx.js
• static/v3/venue-scene-3d.js
• tests/test_plugin_runtime_idempotence.py