Repository navigation
Sound: adaptive layered background music (complexity & laterality) - #168
Merged
Merged
Conversation
Import the six-layer kit (pad→extra, D Dorian 72 BPM), merge the manifest, and drive gain-only loop-synced layers from Complexity and Laterality with bar-quantized equal-power fades. Pad loads with the base kit; layers 2–5 lazy-load after the first card. Complexity 4–5 caps textures at one and ducks the bed −4 dB. Raise audio budget checks to 12 MiB native / 5 MiB web; log caught audio errors. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgqf Agent-Run: https://cursor.com/agents/bc-f4c1505b-7911-5a04-9355-231f3244705e Co-authored-by: gassius <gassius@users.noreply.github.com>
B1: drop web-only LPF/width so preview matches native (gain + arp swap only). Rename soundLayers.web.ts → soundLayerWebGraph.ts so Metro does not shadow the shared soundLayers module. B2: that shadow made createLayerScheduler undefined on web and crashed E2E settle (run 41680652591). Fixed by the rename — no rebaseline. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgqf Agent-Run: https://cursor.com/agents/bc-f4c1505b-7911-5a04-9355-231f3244705e Co-authored-by: gassius <gassius@users.noreply.github.com>
B1: drive createWebLayerGraph + createAdaptiveLayerGroup and assert the GainNode / player volumes they actually apply for every C×L, plus no BiquadFilter/splitter/merger. B3: pause clears step/snap timers but keeps fade state; resume continues or finishes the fade so the mix cannot freeze after backgrounding. stop() clears the snap timer. Tests cover step and snap paths. Minors: web resume restores masterGain; drop stale LPF/width wording. Agent: GasNet Implementer Agent-Ticket: 1amjtrexgqf Agent-Run: https://cursor.com/agents/bc-f4c1505b-7911-5a04-9355-231f3244705e Co-authored-by: gassius <gassius@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Agent: GasNet ImplementerClient E2E screenshots (deterministic Expo web export)
Baseline diff summary (report-only)
|
There was a problem hiding this comment.
Agent: Pull Request Reviewer
Verdict: Request changes
Head: 95d3859edea66064203517156f650af15c44a12e
Blockers
-
<
8000
li>B1 — Web gets audio processing native doesn't have (
- B2 — Failing check
Client E2E screenshots(run 41680652591, job 126073167746). Don't approve while a check is red. Fix: find the cause and fix it, or update the baselines on purpose with a reason. Don't weaken the gate.
apps/client/lib/soundLayers.web.ts:85-107, :205-207). The web graph sends the layers through a BiquadFilter lowpass (2.5–16 kHz, driven by laterality) and a mid/side ChannelSplitter/Merger stereo-width stage. Native only does gain + the arp swap; soundLayers.ts:79 says "filter+width may be N/A on native", and sound.ts says expo-audio has no BiquadFilter or stereo width. So on web, laterality changes tone colour and stereo width, and on Android/iOS it doesn't. That is behaviour only web has, which the native-parity rule doesn't allow. Fix: take the filter and width nodes out of the web chain (layer Gain → group Gain → destination), and on both platforms drive only the per-layer gains, extraGainDb and the arp/arp-bright swap. Drop lowpassHz/width from LATERALITY_MIX, or mark them unused on every platform. Check: a unit test that the web graph creates no BiquadFilter or ChannelSplitter nodes, and that web and native produce the same per-layer gain targets for laterality 1–5.
Minors (in-PR)
soundLayers.web.ts:105-106: the "invert-ish side" wiring sends the same side signal to both L and R, so this isn't real M/S. It goes away once B1 is fixed.
CI
API, Client (Expo), Workflow lint, E2E gate, Vercel: pass. Client E2E screenshots: fail.
Evidence
I read the diff with gh pr diff and the head files with gh api contents?ref=95d3859e. Native players call release() on teardown (soundLayerPlayers.ts:165,308). I found no sudo in the scripts, and the asset licence/provenance is documented in docs/audio/README.md (ElevenLabs, existing assets). Not blocking: the 10-minute device drift log and Carlos's ear sign-off.
There was a problem hiding this comment.
Agent: Pull Request Reviewer
Verdict: Request changes
Head: ec49e079867732e47460cfbeb5b18ee5c472eb1c
Blockers
- B1: the parity test doesn't test anything (
apps/client/lib/soundLayers.test.ts:251-268). It just compareslayerTargets(c,l)with itself, so it would still pass if the filter or width stages came back. The code fix is right:soundLayerWebGraph.ts:78-113is now gain → group gain → destination, with nosetColour(:179). Fix: buildcreateWebLayerGraphon a fake AudioContext and check that it never callscreateBiquadFilter/createChannelSplitter/createChannelMerger. Then drive it andcreateAdaptiveLayerGroup(with fake players) and check that both give the same per-layer gains for C×L 1–5. - B3: native layers freeze if you pause during a fade (
soundLayers.ts:303-304,:340). On native the fade runs on a timer. Ifpause()lands mid-fade,tickreturns early andfadingstays true. After that, everyfireBarjust reschedules itself, so Complexity/Laterality never change the mix again. Going to the background mid-fade is enough to cause it. Fix: keep the tick timer id, clear it inpause/stop, and setfading=false(or restart the fade on resume). Add a test for pause → resume → next bar applies. The snap timer at:330has the same leak:stop()doesn't clear it.
Minors (in-PR)
soundLayerWebGraph.ts:308:resume()sets the gain to 1 and drops the duck/master gain (sound.web.ts:486-487). Restore the last master gain instead.soundLayers.ts:79anddocs/audio/README.md:126still describe laterality as low-pass/width. Update or remove thoseLATERALITY_MIXfields.
CI
Everything passes at ec49e07, including Client E2E screenshots (run 41682134075). The baselines job was skipped.
Evidence
- The delta from 95d3859 touches only
sound.ts,sound.web.ts,soundLayerWebGraph.tsandsoundLayers.test.ts. The PR has no png or baseline files. The E2E fix is the Metro.webrename, guarded by the test at:270-286. - Whole PR, these look fine:
- AppState and a11y listeners are removed (
sound.ts:264-274). - Players are released (
soundLayerPlayers.ts:165,308). - The bar timer is cleared.
- No per-frame setState.
- Music-off turns the layers off (
sound.web.ts:709). - Layers are about 5.2 MB native / 2.0 MB web, within the budget.
- No sudo; the intro logo is untouched.
- The interruption gate is manual by design (
sound.ts:407-413).
- AppState and a11y listeners are removed (
There was a problem hiding this comment.
Agent: Pull Request Reviewer
Verdict: Approve
Head: d86636be0c9be00bbc4c05aaf750cdb53d92f474 (delta from ec49e07, one fast-forward commit)
Blockers
None. Both earlier blockers are fixed:
- B1 fixed (
apps/client/lib/soundLayers.test.ts:361). The test builds the realcreateWebLayerGraphon a fake AudioContext and checks that it makes 0 biquad, splitter or merger nodes. For every C×L 1–5 it reads the GainNode values the graph applies and compares them with the nativecreateAdaptiveLayerGroupplayer volumes and withlayerTargets. The test would now fail if a filter or width stage came back, or if the web gain differed from native. - B3 fixed (
soundLayers.ts:255-273,322-365,455-499).pause()clears the step timer and the snap timer but keepsstepFade/snap state.resume()continues the fade, or finishes it if the remaining snap time has already passed.stop()andfinishFadeclear all fade timers.- New tests drive these paths on a fake clock:
:476(step fade, then the next bar still applies),:516(snap path) and:548(stop clears the snap timer).
Minors
None open.
soundLayerWebGraph.ts:292,312:resume()now restores the master/duck gain. Tested at:588.- The low-pass/width wording is gone from
soundLayers.ts:79-91anddocs/audio/README.md:126.setColourwas removed from both platforms.
CI
All required checks pass at d86636b. Client E2E screenshots took 2m28s (run 41683920278). The baselines and full-screenshot jobs were skipped.
Evidence
The compare shows one commit ahead with 5 files changed. The delta adds no web-only behaviour, no new timer leaks and no sudo. The old test was replaced by a stronger one, so no test got weaker.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ClickUp: https://app.clickup.com/t/1amjtrexgqf
Adds the 6-layer D Dorian music pack (assets sha256 a9f2407b3328763e712d893b5133932dafa067b53af8ae43b45907cdbb7859b2) and a loop-synced layer group driven by Complexity and Laterality.
console.warninstead of silent catches.scripts/measure-layer-drift 8000 .mjs), then Carlos signs off by ear.Co-authored-by: gassius gassius@users.noreply.github.com