Skip to content

refactor(stems): lift #audio shim state into state.js SH container - #31

Merged
byrongamatos merged 1 commit into
mainfrom
feat/es-module-lift-shim-state
Jul 8, 2026
Merged

byrongamatos merged 1 commit into
mainfrom
feat/es-module-lift-shim-state

Conversation

@byrongamatos

@byrongamatos byrongamatos commented Jul 8, 2026 •

Copy link
Copy Markdown
Collaborator

What

Step 8 — first keystone of the entangled-core split. Lifts the 7 reassigned shim scalars out of main.js's IIFE scope into an SH container in state.js:

shimsInstalled, shimsUsable, coreCurrentTimeDesc, corePausedDesc, coreDurationDesc, coreNativePlay, coreNativePause

Same rationale as the S container (step 4b): ES imports are read-only bindings, so reassigned scalars can't be export let — they need SH.x = …. These are read across the transport layer and onSongReady (SH.shimsUsable gates the sloppak takeover), so they must be shared, not private to the future shims module.

Pure state-lift, no behavior change. No function moved yet — this unblocks extracting the transport+shims layer as a module (a later PR) without recreating the read-only-binding problem.

Tests

Full suite green (npm test, 41 pass); tests/test_manifest.py green. Codex preflight: 0 issues. Rewrite audit: 0 double-prefixes, 0 bare refs left, no key/shorthand/string-literal collisions.

On-device smoke pending — this touches the transport-takeover path (SH.shimsUsable gate), so verify playback + transport still work.

Summary by CodeRabbit

  • Bug Fixes
    • Improved the stability of audio playback controls, reducing issues that could affect play/pause behavior and song readiness handling.
    • Made transport setup more reliable so playback shims are applied consistently without interfering with normal playback flow.

Move the 7 reassigned shim scalars (shimsInstalled, shimsUsable, and the
captured coreCurrentTimeDesc/corePausedDesc/coreDurationDesc/coreNativePlay/
coreNativePause) out of main.js's IIFE scope into an SH container in state.js,
same rationale as S: ES imports are read-only bindings, so reassigned scalars
need a container to mutate cross-module. These are read across the transport
layer + onSongReady (SH.shimsUsable gates the takeover), so they must be shared,
not module-private. Keystone for extracting the transport/shims layer next.
Pure state-lift, no behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 12:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is a refactor-only “state lift” that moves the #audio transport-shim mutable scalars out of src/main.js’s IIFE scope into a shared SH container exported from src/state.js, matching the existing S container pattern and enabling future module extraction without running into read-only ES import bindings.

Changes:

  • Added export const SH = { … } in src/state.js to hold shim install/usable guards plus captured core descriptors and native play/pause functions.
  • Updated src/main.js to import SH and replace all former local shim scalars with SH.* accesses/mutations.
  • Updated the onSongReady takeover gate to use SH.shimsUsable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/state.js Introduces the new shared SH container for transport-shim mutable state.
src/main.js Replaces local shim scalar variables with SH.* to centralize transport-shim state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 58ddf9cf-5276-4dff-b381-1c48cadbf522

📥 Commits

Reviewing files that changed from the base of the PR and between 52f0d90 and f028b3a.

📒 Files selected for processing (2)
  • src/main.js
  • src/state.js

📝 Walkthrough

Walkthrough

Audio transport shim state (install/usable flags, captured property descriptors, and native play/pause bindings) is moved from module-scoped local variables in src/main.js into a new shared SH object exported from src/state.js, with shim installation, delegation, and song-ready logic updated to read/write SH instead of locals.

Changes

Transport shim state refactor

Layer / File(s) Summary
SH state container definition
src/state.js
Exports a new SH object with shimsInstalled, shimsUsable, coreCurrentTimeDesc, corePausedDesc, coreDurationDesc, coreNativePlay, and coreNativePause fields.
Shim installation and import wiring
src/main.js
Imports SH; installAudioShims() guards re-entry via SH.shimsInstalled, captures descriptors/native bindings onto SH, and sets SH.shimsUsable after install.
Shim delegation to SH-stored state
src/main.js
Native delegate helpers and #audio currentTime/paused/duration/play/pause shims delegate to SH-stored descriptors/functions when sloppak mode is inactive.
Song-ready gating on SH.shimsUsable
src/main.js
onSongReady() checks SH.shimsUsable instead of the removed local variable before taking over playback control.

Estimated code review effort: 2 (Simple) | ~15 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 clearly and concisely describes the main refactor: moving #audio shim state into the shared SH container in state.js.
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/es-module-lift-shim-state

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

@byrongamatos
byrongamatos merged commit 1c9730f into main Jul 8, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants