feat(metronome): volume control with balanced beats by Dandiggas · Pull Request #43 · Dandiggas/MusiciansPracticeApp

Dandiggas · 2026-04-17T23:57:50Z

Summary

Closes #42. Adds a master volume slider to the metronome and retunes the accent / unaccented base gain ratio so both beats are clearly audible at the default volume.

Volume slider under the BPM slider with a Phosphor speaker icon that swaps between SpeakerX / SpeakerLow / SpeakerHigh based on level. Persists globally to localStorage under practice:metronome-volume.
Master GainNode owned by MetronomeEngine. Live slider drag fades smoothly via a 15 ms linearRampToValueAtTime with an explicit setValueAtTime anchor (cross-browser safe, defensive against the older Safari ramp-start quirk).
Square-law perceptual curve (gain = volume²) so the slider feels natural rather than bottom-heavy.
Accent / unaccent retune from 1.0 / 0.5 to 1.0 / 0.75 — the core fix for the ticket's "other beats are too quiet to hear clearly" complaint.
3 ms attack ramp on every click envelope (setValueAtTime(0, t) → linearRampToValueAtTime(peak, t + 0.003) before osc.start(t)) to kill the DC-offset click/pop that was surfaced by the new accent ratio.
Defense at the engine boundary: setVolume coerces NaN / ±Infinity to 0 before touching AudioParam.gain.value.
Store clamps on save AND read, so corrupt or hand-edited localStorage values fall back to the default.

Files touched

src/lib/practice-session-store.ts — added getMetronomeVolume / saveMetronomeVolume
src/lib/audio/metronome-engine.ts — master gain node, setVolume, attack ramp, retune, named constants
src/components/studio/VolumeIcon.tsx — new three-state speaker icon helper
src/components/studio/MetronomeWidget.tsx — slider row under BPM
src/app/practice-timer/page.tsx — state + hydrate effect + mirror-to-engine effect
3 new test files (store, engine, widget) and one mock extension on the existing page test
Docs: spec + plan under docs/superpowers/

Test plan

Unit tests: npm test → 45/45 passing (7 store + 11 engine + 5 widget + 22 pre-existing page)
TypeScript: npx tsc --noEmit → clean
Manual QA on /practice-timer:
- Volume slider renders under BPM slider, icon swaps at 0 / < 0.5 / ≥ 0.5
- Live slider drag fades smoothly (no zipper noise)
- Volume = 0 → silence; icon shows SpeakerX
- Reload restores last-saved volume; first click plays at that volume
- First-beat pop artifact eliminated after the 3 ms attack-ramp harden commit
- Switching instrument keeps volume (global, not per-instrument)

Out of scope

Dashboard layout regression (unrelated to this feature) is being tracked separately.

🤖 Generated with Claude Code

…elpers Persists a global metronome volume (0..1) to localStorage under "practice:metronome-volume" using the existing readJson/writeJson pattern. Clamps on save AND on read so corrupt or hand-edited values fall into range. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds masterGain to MetronomeEngine. Each per-click envelope gain now connects to masterGain instead of AudioContext.destination. Lays the plumbing for a user-controlled master volume; no audible change yet. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Address code review gaps on eb43aa5: - Strengthen the "masterGain exists" test so it can actually distinguish pre-change from post-change state (asserts the first gain is the master + routes to destination exactly once). - Add coverage for masterGain.disconnect() in stop() and for a fresh masterGain being created on re-start. - Add an inline comment explaining why playClick keeps the "?? destination" fallback alongside the masterGain reference. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

setVolume(v) defends against non-finite values (NaN/Infinity coerce to 0), clamps to [0,1], stores the result, and — when the engine is running — schedules a 15 ms linear ramp on masterGain to v² (square-law perceptual curve). When not running, the value is cached and applied when start() next creates masterGain. Non-finite guard is defense at the engine boundary: every caller (localStorage, UI, programmatic) converges here before touching AudioParam.gain.value where NaN is undefined behavior. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Code-review follow-up on cbbd9f1: - setVolume now calls setValueAtTime(currentValue, now) before linearRampToValueAtTime. Without the anchor, the ramp start time is implementation-defined (historically buggy on older Safari) because the initial gain.value write in start() is not an event-list entry. Explicit anchor makes ramp semantics deterministic across browsers. - Extract 0.015 to a named private readonly MASTER_RAMP_TIME field alongside SCHEDULE_AHEAD_TIME and LOOKAHEAD, matching the codebase's existing constant-naming pattern. - Extend the test mock and strengthen the ramp assertion to verify the anchor is present and carries the current gain value. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Fixes the ticket's core complaint that non-downbeat clicks are too quiet to hear clearly. Old ratio placed unaccented beats at -6 dB vs accent (perceived half-loudness); new ratio is ~-2.5 dB, so the accent still clearly reads as the downbeat but the other beats are part of the same pattern. Scales with the user's master volume. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…t scheduler tuning Code-review follow-up on 6f23d4d: - Inline comment on the accent/unaccent base gain line in playClick documents the ratio origin (~-2.5 dB, issue #42) so future readers see the intent without hunting the PR history. - Preflight length assertion on the unaccent test so a future drop in SCHEDULE_AHEAD_TIME below the synthetic 0.05 s interval fails loudly instead of silently passing against an undefined gain node. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Renders a native range input (0..1, step 0.01) directly under the BPM slider with a Phosphor speaker icon that switches between SpeakerX / SpeakerLow / SpeakerHigh based on level. Slider styling mirrors the BPM slider for visual consistency. Wired via new volume / onVolumeChange props; state owner lives in the page (Task 6). Also extracts VolumeIcon into its own tiny component since three-way icon switching is more readable as a dedicated unit and may be reused when other widgets gain volume controls. Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Hydrates metronomeVolume from localStorage on mount, persists on change, applies the saved value before the first click on engine start, and mirrors volume changes to the live engine via a useEffect parallel to setBpm / setBeatsPerMeasure / setOnBeat. Completes the volume-control feature end-to-end. Test mocks extended to include setVolume on the engine mock and getMetronomeVolume / saveMetronomeVolume on the store mock, so the existing 22 page tests keep passing against the new call paths. Closes #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Manual QA surfaced a loud click/pop on the very first beat after pressing Start. Root cause: the per-click envelope hard-stepped from 0 to peak at the same instant the oscillator started, producing a DC-offset click artifact. The artifact pre-existed on main, but the unaccent retune (0.5 → 0.75) in this PR made the contrast sharper, so the first-beat pop stands out against the more-balanced subsequent beats. Fix: schedule the attack as setValueAtTime(0, time) + linearRampToValueAtTime(peak, time + 0.003) BEFORE osc.start(time), so the first sample lands inside a smooth 3 ms ramp instead of a step function. 3 ms is imperceptible as a volume change but eliminates the pop. Also: hoist the 3 ms attack to a named CLICK_ATTACK_TIME constant alongside MASTER_RAMP_TIME / SCHEDULE_AHEAD_TIME / LOOKAHEAD, and strengthen the accent-click test to verify the anti-pop invariant (attack ramp scheduled before oscillator.start via jest invocation order). Refs #42. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a global metronome master volume control (UI + persistence) and adjusts click envelope/levels so unaccented beats are more audible while avoiding audible pops.

Changes:

Persist metronome volume to localStorage and hydrate it on /practice-timer.
Add a master GainNode in MetronomeEngine with square-law curve and a short ramp for smooth live adjustments.
Update the metronome widget UI with a volume slider + speaker icon states, and add unit tests across store/engine/widget.

Reviewed changes

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

Show a summary per file

File	Description
frontend/next-app/src/lib/practice-session-store.ts	Adds `getMetronomeVolume` / `saveMetronomeVolume` with clamping.
frontend/next-app/src/lib/practice-session-store.test.ts	Adds unit tests for metronome volume persistence behavior.
frontend/next-app/src/lib/audio/metronome-engine.ts	Introduces master gain routing, `setVolume`, attack ramp, and accent balance adjustments.
frontend/next-app/src/lib/audio/metronome-engine.test.ts	Adds unit tests for routing, volume curve/ramping, and per-click envelope behavior.
frontend/next-app/src/components/studio/VolumeIcon.tsx	New helper component to display volume state via Phosphor icons.
frontend/next-app/src/components/studio/MetronomeWidget.tsx	Adds a volume slider row under the BPM slider.
frontend/next-app/src/components/studio/MetronomeWidget.test.tsx	Adds widget tests for slider wiring and icon state selection.
frontend/next-app/src/app/practice-timer/page.tsx	Wires volume state into the page, hydrates/saves it, and mirrors it into the engine.
frontend/next-app/src/app/practice-timer/tests/page.test.tsx	Extends mocks for the new store + engine volume APIs.

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

Copilot · 2026-04-18T00:07:50Z

+const clamp01 = (n: number): number => Math.min(1, Math.max(0, n));
+
+export const getMetronomeVolume = (): number | null => {
+  const record = readJson<MetronomeVolumeRecord>(METRONOME_VOLUME_KEY);
+  if (!record || typeof record.volume !== "number") return null;
+  return clamp01(record.volume);
+};
+
+export const saveMetronomeVolume = (volume: number): void => {
+  writeJson(METRONOME_VOLUME_KEY, {
+    volume: clamp01(volume),
+    updatedAt: new Date().toISOString(),
+  } satisfies MetronomeVolumeRecord);


clamp01 doesn’t handle NaN: clamp01(NaN) returns NaN, and saveMetronomeVolume(NaN) will serialize as { volume: null, ... } (since JSON.stringify(NaN) becomes null). That means a non-finite input won’t actually clamp to 0 and will read back as null (defaulting the UI instead of muting). Consider coercing non-finite values to 0 in clamp01 (e.g., via Number.isFinite) or in saveMetronomeVolume/getMetronomeVolume before clamping, to match the engine’s non-finite defense.

Copilot · 2026-04-18T00:07:51Z

+      // Anchor the ramp with an explicit setValueAtTime. linearRampToValueAtTime ramps from
+      // the value of the previous scheduled event — without this anchor, a bare `.gain.value = v`
+      // write (done in start()) isn't an event-list entry, and the ramp start-time is
+      // implementation-defined (historically buggy on older Safari).
+      this.masterGain.gain.setValueAtTime(this.masterGain.gain.value, now);
+      this.masterGain.gain.linearRampToValueAtTime(target, now + this.MASTER_RAMP_TIME);


setVolume schedules a new setValueAtTime + linearRampToValueAtTime each time it’s called, but it doesn’t cancel any previously scheduled automation events. During a live slider drag, older ramps that are still scheduled in the future can fire after a newer update and cause gain discontinuities / “rubber-banding”. Consider calling cancelScheduledValues(now) (or cancelAndHoldAtTime(now) where available) on masterGain.gain before anchoring and ramping to the new target.

Suggested change

// Anchor the ramp with an explicit setValueAtTime. linearRampToValueAtTime ramps from

// the value of the previous scheduled event — without this anchor, a bare `.gain.value = v`

// write (done in start()) isn't an event-list entry, and the ramp start-time is

// implementation-defined (historically buggy on older Safari).

this.masterGain.gain.setValueAtTime(this.masterGain.gain.value, now);

this.masterGain.gain.linearRampToValueAtTime(target, now + this.MASTER_RAMP_TIME);

const gainParam = this.masterGain.gain;

const holdableGainParam = gainParam as AudioParam & {

cancelAndHoldAtTime?: (cancelTime: number) => AudioParam;

};

// Replace any future automation so older slider updates can't "rubber-band"

// the gain after a newer setVolume() call.

if (typeof holdableGainParam.cancelAndHoldAtTime === 'function') {

holdableGainParam.cancelAndHoldAtTime(now);

} else {

gainParam.cancelScheduledValues(now);

}

// Anchor the ramp with an explicit setValueAtTime. linearRampToValueAtTime ramps from

// the value of the previous scheduled event — without this anchor, a bare `.gain.value = v`

// write (done in start()) isn't an event-list entry, and the ramp start-time is

// implementation-defined (historically buggy on older Safari).

gainParam.setValueAtTime(gainParam.value, now);

gainParam.linearRampToValueAtTime(target, now + this.MASTER_RAMP_TIME);

Dandiggas and others added 10 commits April 17, 2026 23:10

Dandiggas requested a review from Copilot April 18, 2026 00:03

Copilot started reviewing on behalf of Dandiggas April 18, 2026 00:04 View session

Dandiggas merged commit df625d3 into main Apr 18, 2026
7 checks passed

Copilot AI reviewed Apr 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(metronome): volume control with balanced beats#43

feat(metronome): volume control with balanced beats#43
Dandiggas merged 10 commits into
mainfrom
feature/metronome-volume-control

Dandiggas commented Apr 17, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-      // Anchor the ramp with an explicit setValueAtTime. linearRampToValueAtTime ramps from
-      // the value of the previous scheduled event — without this anchor, a bare `.gain.value = v`
-      // write (done in start()) isn't an event-list entry, and the ramp start-time is
-      // implementation-defined (historically buggy on older Safari).
-      this.masterGain.gain.setValueAtTime(this.masterGain.gain.value, now);
-      this.masterGain.gain.linearRampToValueAtTime(target, now + this.MASTER_RAMP_TIME);
+      const gainParam = this.masterGain.gain;
+      const holdableGainParam = gainParam as AudioParam & {
+        cancelAndHoldAtTime?: (cancelTime: number) => AudioParam;
+      };
+      // Replace any future automation so older slider updates can't "rubber-band"
+      // the gain after a newer setVolume() call.
+      if (typeof holdableGainParam.cancelAndHoldAtTime === 'function') {
+        holdableGainParam.cancelAndHoldAtTime(now);
+      } else {
+        gainParam.cancelScheduledValues(now);
+      }
+      // Anchor the ramp with an explicit setValueAtTime. linearRampToValueAtTime ramps from
+      // the value of the previous scheduled event — without this anchor, a bare `.gain.value = v`
+      // write (done in start()) isn't an event-list entry, and the ramp start-time is
+      // implementation-defined (historically buggy on older Safari).
+      gainParam.setValueAtTime(gainParam.value, now);
+      gainParam.linearRampToValueAtTime(target, now + this.MASTER_RAMP_TIME);

Conversation

Dandiggas commented Apr 17, 2026

Summary

Files touched

Test plan

Out of scope

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants