Migrate Sound Module to Conductor#796
Open
Akshay-2007-1 wants to merge 12 commits into
Open
Conversation
Scraps the earlier plugins-repo sound-io plugin pair in favor of the pattern confirmed against rune's migration (PR #765): SoundModulePlugin declares its own channelAttach directly, and modules/src/tabs/Sound implements IPlugin/SoundTabRpc as the host-side counterpart, talking over Conductor's makeRpc helper - no separate runner/web plugin package needed at all. Also fixes plotly's draw_sound_2d, the one other consumer of bundle-sound's types, for the Wave type's redesign from a plain (t: number) => number to an async generator (so that stepping and nested user closures evaluate correctly on the CSE machine). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the midi and sound bundles to integrate with the @sourceacademy/conductor framework, enabling generator-based wave functions that preserve debugger stepping and breakpoints. Audio I/O operations are offloaded to a host-bridged RPC interface (SoundTabRpc) since browser audio APIs are unavailable in the sandboxed runner Worker, and the Sound tab is rewritten as a React-based plugin. Review feedback highlights critical performance issues in consecutively and simultaneously where using reduce to nest generator functions leads to severe performance degradation at 44100 Hz; refactoring these to use flat loops is recommended. Additionally, a guard should be added to linear_decay to prevent NaN propagation when the decay period is zero.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Retires stereo_sound as a separate bundle/tab entirely. Sound becomes
{leftWave, rightWave, duration} always; "mono" isn't a separate type,
it's just the common case where leftWave === rightWave (same
reference), produced by make_sound. make_stereo_sound builds a
genuinely stereo Sound from two different waves. Every combinator
(consecutively/simultaneously/adsr/phase_mod/stacking_adsr/
instruments) now operates on this one shape via small per-channel
helpers (joinWaves/sumWaves/adsrWave/phaseModWave), so composing a
"mono" sound with a stereo one is a non-issue - there's nothing to
convert, and none of stereo_sound's oscillator/envelope math is
duplicated a second time the way it was as a separate bundle.
get_wave/get_left_wave/get_right_wave: get_wave keeps meaning "the"
wave (== left channel), so existing SICP-style code (play(sine_sound(...)),
get_wave(sound)) keeps working unchanged for the common mono case.
Adds a lower Wave-returning layer alongside the existing Sound layer
(sine_wave/square_wave/triangle_wave/sawtooth_wave/noise_wave/
silence_wave), with sine_sound etc. as thin convenience wrappers
(sine_sound = (freq, dur) => make_sound(sine_wave(freq), dur)).
record()/record_for() use however many channels the input device
actually has - a mono microphone (the common case) produces a Sound
whose left and right channels are the same wave; no separate
record_stereo. play/samplesToSound skip redoing work for a mono
Sound (sampled once, not twice) by checking leftWave === rightWave.
New stereo-specific operations: make_stereo_sound, play_waves,
pan, pan_mod, squash, get_left_wave, get_right_wave.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nification
Continuation of the previous commit (which only picked up the
stereo_sound/StereoSound deletions due to a failed multi-pathspec git
add) - this is the actual Sound={leftWave,rightWave,duration} rework
of the sound bundle and its tab described there.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…igrate-sound Resolved conflicts by keeping the already-Conductor-migrated code (midi, sound, and stereo_sound's retirement) over conductor-migration's stale pre-migration content, same pattern as feat/migrate-binary-tree and feat/migrate-midi. Also re-applies the noImplicitOverride fix and the midi scale-test fix that landed separately on feat/migrate-midi (this branch has its own independent midi history). yarn.lock resolved per the review feedback on PR #795 (reset to origin/conductor-migration's copy first, then reinstalled, so only new/changed dependencies actually move instead of picking up whatever was stale on this branch); .yarnrc.yml also gets @sourceacademy/conductor added to npmPreapprovedPackages for the same reason as PR #795. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#795 (fix/conductor-dependency-pin) is about to merge into conductor-migration first, establishing @sourceacademy/conductor in the yarn catalog and npmPreapprovedPackages. Switching midi/sound/ tab-Sound's package.json over to "catalog:" now (matching #795 verbatim) avoids re-doing this exact reconciliation the next time this branch merges conductor-migration in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses Gemini's high-priority review comments on PR #796: reduce() was nesting joinWaves/sumWaves closures sounds.length deep, so sampling near the end of a long chain meant up to N nested async generator delegations per sample at 44100 Hz - real overhead for async generators that plain synchronous functions (the pre-migration implementation) never had. Rewritten as a flat scan that picks the active sound directly and yield*s into it once, keeping delegation depth O(1) regardless of how many sounds are combined, while still threading through user closures correctly. Also fixes a real (if narrow) NaN: linear_decay(0) computed 1 - 0/0 when release_ratio (or a future caller with decay_ratio) is exactly 0, corrupting the sample at that instant. Sample rate/instrument functions never happened to hit it because of how the surrounding branch conditions are structured, but adsr's release branch can. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Akshay-2007-1
marked this pull request as ready for review
July 16, 2026 05:24
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Akshay-2007-1
added a commit
that referenced
this pull request
Jul 16, 2026
Matches the same fix already applied on #795 and #796: adds @sourceacademy/conductor to the yarn catalog and npmPreapprovedPackages, and switches midi's own package.json to "catalog:" instead of a literal "^0.7.0" pin, so this PR doesn't leave midi on the old convention once #795 lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
WalkthroughThe PR migrates MIDI and sound bundles to Conductor plugins, changes sound processing to asynchronous stereo generators, adds browser audio RPC integration, removes legacy stereo-sound and audio-tab artifacts, and updates tests, package wiring, and Plotly sampling. ChangesConductor migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Program
participant SoundModulePlugin
participant SoundFunctions
participant SoundTabPlugin
participant BrowserAudio
Program->>SoundModulePlugin: call sound module method
SoundModulePlugin->>SoundFunctions: convert Conductor values
SoundFunctions->>SoundTabPlugin: request playback or recording
SoundTabPlugin->>BrowserAudio: play samples or capture microphone
BrowserAudio-->>SoundTabPlugin: completion or recorded PCM
SoundTabPlugin-->>SoundFunctions: return RPC result
SoundFunctions-->>SoundModulePlugin: return Conductor value
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundles/plotly/src/functions.ts (1)
264-286: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winCap waveform sampling to a display-sized point count.
This performs 44,100 serial generator drains per second of audio. Generator/Promise overhead and oversized Plotly datasets can freeze the UI for ordinary multi-second sounds. Sample using a stride capped to a reasonable maximum point count.
🤖 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 `@src/bundles/plotly/src/functions.ts` around lines 264 - 286, Update draw_sound_2d so waveform sampling uses a stride that limits the generated time_stamps and channel arrays to a reasonable display-sized maximum point count instead of iterating once per 44,100 Hz sample. Apply the stride consistently to the sampled time, wave invocation, and loop bounds while preserving the existing generator-draining behavior and output rendering.
🧹 Nitpick comments (1)
src/bundles/midi/src/__tests__/functions.test.ts (1)
71-89: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression cases for invalid bare notes.
Cover
B#,E#,Cb, andFb, plus lowercase normalization, soadd_octave_to_notecannot return values rejected by the canonical parser.🤖 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 `@src/bundles/midi/src/__tests__/functions.test.ts` around lines 71 - 89, Add regression tests in the add_octave_to_note suite for invalid bare notes B#, E#, Cb, and Fb, asserting each throws the canonical validation error rather than producing an octave-appended value. Also add lowercase-input cases to verify supported lowercase notes are normalized correctly while invalid lowercase accidental forms remain rejected.
🤖 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.
Inline comments:
In `@src/bundles/midi/src/functions.ts`:
- Around line 150-159: Update add_octave_to_note in
src/bundles/midi/src/functions.ts:150-159 to use parseNoteWithOctave as the
canonical validator, reject inputs that already contain octave digits, and
normalize lowercase note names before returning. Extend the add_octave_to_note
tests in src/bundles/midi/src/__tests__/functions.test.ts:71-89 with B#, E#, Cb,
Fb, and lowercase-normalization cases.
In `@src/bundles/sound/src/functions.ts`:
- Around line 586-596: Update consecutively and the corresponding simultaneous,
transformation, and phase-modulation functions in
src/bundles/sound/src/functions.ts at lines 586-596, 623-633, 677-694, and
736-742 so mono inputs reuse the same generated wave by reference for both
channels, while stereo behavior remains unchanged. Add identity assertions in
src/bundles/sound/src/__tests__/sound.test.ts at lines 185-280 for mono
composition and 331-339 for mono ADSR preservation.
- Around line 776-786: Update pan to evaluate the squashed source once per
timestamp and reuse that sample for both stereo channels instead of
independently sampling through gainWave. In the modulation path around the
related functions at src/bundles/sound/src/functions.ts:803-817, share both the
source sample and modulation amount between channels. Add stateful-wave and
invocation-count regression cases at
src/bundles/sound/src/__tests__/sound.test.ts:298-329.
- Around line 431-446: Update the playback state flow around the sound-start
function and stop() in src/bundles/sound/src/functions.ts lines 431-446 to track
a generation token, incrementing it when playback starts or stops and clearing
globalVars.isPlaying in the promise finally handler only when its token is still
current. Add coverage in src/bundles/sound/src/__tests__/sound.test.ts lines
88-183 for stop-A/start-B/settle-A ordering, verifying stale playback completion
cannot clear the newer playback state.
- Around line 283-319: Update record and the related recording API around the
returned stop callbacks to synchronously reserve recording state before
scheduling any signals, rejecting calls while a recording is pending or active.
Reset the state only after stopRecording() settles, including failure paths, and
make each stop callback idempotent so repeated invocations cannot trigger
duplicate stops or signals.
- Around line 94-100: Update validateDuration to reject non-finite numeric
durations in addition to non-number and negative values. Use a finite-number
check so NaN and both infinities throw the existing appropriate validation error
before playback or allocation proceeds.
In `@src/bundles/sound/src/index.ts`:
- Around line 4-5: Update the documentation for the Wave contract to state that
a wave accepts time t and returns an async generator whose return value is the
amplitude, replacing the description that it directly returns a number.
- Around line 492-507: Validate each value extracted by the envelope-list
traversal before storing or invoking it as a closure. Update the logic around
envelopeClosures and closure_call_unchecked so non-closure elements are rejected
through the module’s established type-validation/error path, while valid closure
elements retain the existing harmonic processing behavior.
- Around line 95-102: Update closureToWave to validate the result returned by
evaluator.closure_call_unchecked before using result.value as the wave sample:
require the returned DataType to be NUMBER and ensure the runtime value is a
valid number, rejecting or otherwise handling non-number and NaN results
according to the surrounding wave error conventions. Do not rely on the existing
TypeScript cast as runtime validation.
- Around line 136-145: Update the Sound unwrapping return logic around leftWave
and rightWave so that when leftTv and rightTv contain the same closure value,
both channels reuse a single closureToWave result and preserve leftWave ===
rightWave; continue creating separate wrappers for distinct closures.
In `@src/tabs/Sound/src/index.tsx`:
- Around line 100-108: Update requestMicPermission() to stop all tracks on the
existing __mediaStream and clear it before calling
navigator.mediaDevices.getUserMedia. Then reacquire the stream and update
__micGranted as currently handled, ensuring a denied request cannot leave a
reusable previous stream.
- Around line 140-155: Update startRecording in src/tabs/Sound/src/index.tsx:
await the MediaRecorder start event before resolving and reject the promise when
the recorder emits error, while preserving the existing setup and status
transition. Update both affected mock/test sites in
src/tabs/Sound/src/__tests__/index.test.ts (lines 74-82 and 159-163) so start
fires asynchronously and the tests verify startRecording remains pending until
that event.
---
Outside diff comments:
In `@src/bundles/plotly/src/functions.ts`:
- Around line 264-286: Update draw_sound_2d so waveform sampling uses a stride
that limits the generated time_stamps and channel arrays to a reasonable
display-sized maximum point count instead of iterating once per 44,100 Hz
sample. Apply the stride consistently to the sampled time, wave invocation, and
loop bounds while preserving the existing generator-draining behavior and output
rendering.
---
Nitpick comments:
In `@src/bundles/midi/src/__tests__/functions.test.ts`:
- Around line 71-89: Add regression tests in the add_octave_to_note suite for
invalid bare notes B#, E#, Cb, and Fb, asserting each throws the canonical
validation error rather than producing an octave-appended value. Also add
lowercase-input cases to verify supported lowercase notes are normalized
correctly while invalid lowercase accidental forms remain rejected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1116f4c-268c-4ba8-902d-306f053f5896
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (39)
.yarnrc.ymlsrc/bundles/midi/package.jsonsrc/bundles/midi/src/__tests__/functions.test.tssrc/bundles/midi/src/__tests__/index.test.tssrc/bundles/midi/src/conductorAdapters.tssrc/bundles/midi/src/functions.tssrc/bundles/midi/src/index.tssrc/bundles/midi/src/scales.tssrc/bundles/midi/src/utils.tssrc/bundles/midi/tsconfig.jsonsrc/bundles/plotly/src/functions.tssrc/bundles/sound/package.jsonsrc/bundles/sound/src/__tests__/recording.test.tssrc/bundles/sound/src/__tests__/sound.test.tssrc/bundles/sound/src/__tests__/utils.tssrc/bundles/sound/src/functions.tssrc/bundles/sound/src/index.tssrc/bundles/sound/src/play_in_tab.tssrc/bundles/sound/src/protocol.tssrc/bundles/sound/src/riffwave.tssrc/bundles/sound/src/types.tssrc/bundles/sound/tsconfig.jsonsrc/bundles/stereo_sound/manifest.jsonsrc/bundles/stereo_sound/package.jsonsrc/bundles/stereo_sound/src/__tests__/index.test.tssrc/bundles/stereo_sound/src/functions.tssrc/bundles/stereo_sound/src/index.tssrc/bundles/stereo_sound/src/riffwave.tssrc/bundles/stereo_sound/src/types.tssrc/bundles/stereo_sound/tsconfig.jsonsrc/tabs/Sound/index.tsxsrc/tabs/Sound/package.jsonsrc/tabs/Sound/src/__tests__/index.test.tssrc/tabs/Sound/src/index.tsxsrc/tabs/Sound/tsconfig.jsonsrc/tabs/StereoSound/package.jsonsrc/tabs/StereoSound/src/__tests__/index.test.tsxsrc/tabs/StereoSound/src/index.tsxsrc/tabs/StereoSound/tsconfig.json
💤 Files with no reviewable changes (15)
- src/bundles/stereo_sound/tsconfig.json
- src/tabs/StereoSound/src/index.tsx
- src/tabs/StereoSound/tsconfig.json
- src/bundles/stereo_sound/src/tests/index.test.ts
- src/bundles/sound/src/play_in_tab.ts
- src/bundles/stereo_sound/package.json
- src/bundles/stereo_sound/src/riffwave.ts
- src/tabs/StereoSound/package.json
- src/bundles/sound/src/riffwave.ts
- src/bundles/stereo_sound/src/types.ts
- src/bundles/stereo_sound/manifest.json
- src/tabs/Sound/index.tsx
- src/bundles/stereo_sound/src/index.ts
- src/tabs/StereoSound/src/tests/index.test.tsx
- src/bundles/stereo_sound/src/functions.ts
- validateDuration now rejects NaN/Infinity, not just negatives (both used to slip through and crash sampleWave's Float32Array allocation with a raw RangeError instead of a clean module error). - closureToWave validates the closure's result is actually a number before returning it - a student-supplied wave returning e.g. a string previously corrupted playback silently via `as number`. - stacking_adsr validates each envelope-list element is a closure before invoking it, for the same reason. - consecutively/simultaneously/adsr/phase_mod/conductorToSound all preserve the "mono means leftWave === rightWave" invariant again - building both channels independently (even from identical inputs) produced two behaviourally-identical-but-distinct waves, silently doubling sampling work and losing the fast path in play()/etc. - play()/stop() guard against a stale playSamples() completion clobbering a newer play()'s isPlaying state via a generation token (stop-A/start-B/late-settle-A ordering). - tab: requestMicPermission() disposes the previous MediaStream before re-requesting, so a denied re-request can't leave stale tracks running and reusable by startRecording(). - tab: startRecording() now actually awaits MediaRecorder's start event (and rejects on error) instead of resolving as soon as .start() returns, matching the SoundTabRpc contract. Not addressed (flagged as false positive or out of scope for a quick fix, not silently dropped): - The module doc's "a wave returns a number" description is correct as written - it's the student-facing Source-language contract, not the internal async-generator implementation detail (which is already documented separately on the Wave type in types.ts). - Overlapping/concurrent recording sessions (record/record_for only gate on isPlaying, not a dedicated recording-state flag) and pan/pan_mod sampling the shared source/modulator wave twice per channel are both real but non-trivial fixes requiring more substantial state-tracking/sampling-order changes; left for a follow-up rather than a rushed partial fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Akshay-2007-1
changed the base branch from
conductor-migration
to
feat/migrate-midi
July 16, 2026 06:16
…rate-sound Needed after retargeting PR #796's base to feat/migrate-midi (stacking it on top of midi's PR instead of conductor-migration): resolved by keeping sound's already-migrated+CodeRabbit-fixed functions.ts (midi's branch still had the stale pre-migration copy) and re-deleting stereo_sound (midi's branch never touched it, so git saw sound's deletion as conflicting with midi's own unrelated merge-driven edits to it). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e module doc Partial concession on CodeRabbit's doc-comment finding: the primary description stays as the Source-facing "number -> number" contract (that's the actual, correct student experience - the CSE machine threads the async-generator machinery transparently), but adds a one-line pointer to where the internal TS contract is documented, for maintainers reading this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Akshay-2007-1
added a commit
that referenced
this pull request
Jul 16, 2026
…ave_to_note Addresses CodeRabbit's review comment on PR #796 (surfaced there via shared branch history with sound, but the finding is midi's own): add_octave_to_note's inline regex accepted note spellings that noteToValues/parseNoteWithOctave reject elsewhere (B#, E#, Cb, Fb - accidentals that don't exist for those note names), and let lowercase input escape unnormalized through a type assertion. Now validates via parseNoteWithOctave (rejecting digits upfront, since that function alone would accept an octave already being present) and reconstructs using the normalized note name, preserving the original accidental spelling exactly as given. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…l testing Manually testing all three migrated modules (binary_tree, midi, sound) together against a local frontend build surfaced several issues specific to sound, now fixed: - play() threw "Previous sound still playing!" on any overlapping or looped call. Repeated/looped play() calls now queue and play one after another instead (like consecutively, but built up call-by-call rather than pre-combined into one Sound). stop() cancels anything still queued behind the currently-playing sound, not just the current one. - The tab's "Constructing..." status (shown while play() samples a Wave into a buffer - a duration-proportional step that happens entirely before actual playback starts) never actually appeared, since the notification was fire-and-forget and could race the tab's own asynchronous loading. Now a real acknowledged RPC call, awaited before sampling begins. - init_record() was fire-and-forget: it kicked off the permission request but returned immediately, so calling record()/record_for() right after (the natural way to write it) could race the still- pending permission grant and throw even though permission had genuinely just been granted. init_record() now awaits the actual permission result before returning. - record()/record_for()'s returned "sound promise" threw "recording still being processed" until called again later, requiring manual polling. Both now return a promise that genuinely awaits the recording finishing processing instead. - adsr() had no validation that attack_ratio + decay_ratio + release_ratio stays within 1, or that each ratio/sustain_level is a finite number in [0, 1] - silently producing a discontinuous envelope instead of an error. Added validation, with the actual envelope-shaping logic split into an unvalidated adsrTransformer() used internally by piano/violin/cello/trombone/bell, so validating adsr() doesn't retroactively break trombone's second-harmonic envelope (ratios summing to 1.0236, a quirk present since before the Conductor migration - preserved rather than silently changed).
…teardown Found via extensive live testing against a local frontend build: - The Worker running a program is terminated as soon as the script finishes (conduit.terminate(), called after every Run to fix a worker-leak). play() is intentionally fire-and-forget, so a script can finish - and the Worker be killed - well before queued playback has actually started or finished. Sequencing that playback via a Worker-side queue (the previous design) meant anything still queued when the Worker died simply never got its playSamples() RPC sent at all. - functions.ts' play() now dispatches its playSamples() call immediately once sampling finishes, instead of waiting its turn in a local queue - so the RPC always gets sent before the Worker can be torn down. Actual sequencing (so playback doesn't overlap) moves to SoundTabPlugin on the host side, which outlives the Worker: it now owns its own playback queue and a stop-generation counter so a still-queued call correctly gets cancelled by stop(). - SoundTabPlugin.destroy() (called on every Run's teardown) no longer closes the AudioContext or unregisters the tab immediately - both are deferred until whatever's playing (or still queued) finishes naturally, tracked via a dedicated pending-playback counter rather than activeSources.size, which hits 0 momentarily between any one sound ending and the next queued one starting and was otherwise misread as "everything is done," silently killing the AudioContext mid-queue. - notifyConstructing()/playSamples() status updates are now recomputed from combined constructing+active-source state instead of set unconditionally, so an earlier sound finishing can't clobber a later sound's still-in-flight 'constructing' status.
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
1 participant
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.
Summary
soundbundle to the Conductor framework, and unifies it withstereo_sound(retired entirely - no separate bundle/tab): Sound is now always{leftWave, rightWave, duration}internally. "Mono" isn't a separate type, it's just the common case whereleftWave === rightWave(same reference), produced bymake_sound. Every combinator (consecutively/simultaneously/adsr/phase_mod/stacking_adsr/instruments) operates on this one shape via small per-channel helpers, so none of stereo_sound's oscillator/envelope math is duplicated a second time the way it was as a separate bundle.Waves ((t: number) => AsyncGenerator<void, number, undefined>) so that stepping/breakpoints and nested user closures evaluate correctly on the CSE machine.Wave-returning layer alongside the existingSoundlayer (sine_wave/square_wave/triangle_wave/sawtooth_wave/noise_wave/silence_wave), withsine_soundetc. as thin convenience wrappers.record()/record_for()use however many channels the input device actually has - a mono microphone (the common case) produces a Sound whose left and right channels are the same wave; no separaterecord_stereo.make_stereo_sound,play_waves,pan,pan_mod,squash,get_left_wave,get_right_wave(get_wavekeeps meaning "the" wave == left channel, so existing SICP-style code keeps working unchanged for the mono case).SoundModulePlugindeclares its ownchannelAttach, andmodules/src/tabs/SoundimplementsIPlugin/SoundTabRpcdirectly as the host-side counterpart (talking over Conductor'smakeRpchelper, now driving a real 2-channelAudioBuffer), matching the pattern confirmed against the rune migration (PR Migrate Rune Module #765) - no separate runner/web plugin package needed.plotly'sdraw_sound_2d, the only other consumer ofbundle-sound's types, for theWavetype's redesign.Draft while the tab-loading contract (loadTab/tabs, matching rune's PR #765) is still being finalized upstream.
Fixes #766 and #783
Test plan
tscclean forbundle-sound,tab-Sound, andbundle-plotlybundle-soundtests: 41/41 passingtab-Soundtests: 11/11 passingtscacross all bundles/tabs: clean