sound: constructor ordering, tab lookup by name, and a sync-probe crash fix

[Akshay-2007-1]

Summary

Two small, unrelated sound fixes bundled together since both are tiny.

Fixes #828, two low-priority nitpicks found while reviewing csg:

• The channel guard in SoundModulePlugin's constructor ran after super(conduit, [soundChannel], evaluator), so BaseModulePlugin already got handed an [undefined] channels array before the check fired. A throw is legal before super() as long as this isn't touched, so the guard now runs first.
• __ensureTabLoaded picked the tab by position (tabs[0]) instead of by name. It now uses tabs.find(tab => tab === SOUND_TAB_NAME), matching the existing SOUND_TAB_NAME export in protocol.ts, and fails loudly if the tab is ever missing instead of silently.

Also fixes a real crash surfaced while testing csg, related to the same family of issues as py-slang#348/#350 and modules#832:

from sound import (play_wave, sine_wave)

def square(x): return x * x

def doppler(max, t1, t2):
    b = square(t2 - t1) - 2 * (t2 - t1) * t2
    ts = lambda t: (0 if t < t1
                    else square(t - t1) * max / square(t2 - t1) if t < t2
                    else (2 * (t2 - t1) * t + b) * max / square(t2 - t1))
    return lambda w: lambda t: w(t + ts(t))

play_wave(doppler(-0.1, 1, 2)(sine_wave(1000)), 3)

On py2js only, this threw <module function>() needs a frontend round-trip and cannot be called from a synchronous module callback, even though the code is completely valid.

Root cause: closureToWave's sync-fast-path probe (in index.ts) assumed closure_call_sync could only ever fail cleanly, by returning undefined before the closure runs. That assumption breaks here: the composed Python closure gets a real sync-compiled body from py2js, and that body itself calls w (the sine_wave closure passed in as an argument), which needs a genuine host round-trip once it's crossed back into student code this way. There's no way to suspend from inside an already-synchronous body, so it throws instead of returning undefined, and that throw was propagating straight out of the probe and crashing the whole sample instead of being treated as "no sync twin available, fall back to async."

Fixed by wrapping the probe in a try/catch: any throw during the probe is now treated exactly like an undefined result, and the wave falls back to the always-correct async path.

This patches the symptom here in sound so it no longer crashes, but the actual reason w needs a round-trip at all in this case is a still-open gap in py-slang: moduleToPython marks every module closure asyncOnly unconditionally, regardless of whether it actually has a .sync twin. py-slang#353 fixes that at the engine level, giving a module closure crossing into student code (like sine_wave's returned wave here) a real synchronous fast path when one is available, instead of an unconditional round-trip. Once #353 merges, this exact doppler-style composition should stop needing the async fallback at all and start getting the sync fast path for free; this PR's catch just makes sure things don't crash in the meantime (and afterward, for any other closure that genuinely can't go sync).

Test plan

• tsc --noEmit on src/bundles/sound shows no new errors from either change
• eslint on the changed file is clean
• Existing sound test suite doesn't cover either the constructor path or index.ts's closureToWave, matching how prior sync-fast-path fixes in this file (see git history) landed without new tests; no behavior change expected for waves that don't hit this specific nested case

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[Akshay-2007-1]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

The sound plugin now validates its required channel before base initialization and resolves the host tab by the exported sound tab name rather than by position.

Changes

Sound plugin updates

Layer / File(s)	Summary
Sound wiring and tab resolution src/bundles/sound/src/index.ts	Imports SOUND_TAB_NAME, checks the sound channel before super(...), and selects the matching tab during lazy loading.

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

Possibly related PRs

• source-academy/modules#796: Updates Sound bundle wiring and tab-name usage during the Conductor migration.

Suggested reviewers: martin-henz

Poem

I’m a rabbit with sound in my ear,
A named tab now hops into gear.
The channel checks first,
Before bases are versed,
And clean plugin wiring grows clear!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code changes implement #828 by moving the channel guard before super() and resolving the sound tab by SOUND_TAB_NAME.
Out of Scope Changes check	✅ Passed	No unrelated or out-of-scope changes are indicated beyond the two sound-module fixes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title clearly names the two main sound changes and remains concise.
Description check	✅ Passed	The description covers the summary, issue, context, and testing, but it omits the template's type-of-change and checklist sections.

✨ 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 fix/sound-channel-guard-and-tab-lookup

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/sound/src/index.ts`:
- Around line 412-414: Update the tab lookup boundary in the relevant Sound
initialization method: when `__tabLoader.tabs.find(...)` cannot locate
`SOUND_TAB_NAME`, throw or propagate a clear internal error instead of
returning. Ensure callers such as `init_record()` and `play()` observe the
explicit failure and do not continue with tab-dependent operations or retry
silently.

🪄 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 Plus

Run ID: d808ed07-3a2e-451c-87d5-3bc479eefea3

📥 Commits

Reviewing files that changed from the base of the PR and between aa37ecb and 60a70d3.

📒 Files selected for processing (1)

• src/bundles/sound/src/index.ts

[martin-henz]

LGTM