From fed42c7bc08bc279b6568a0115c715a7d9b83a63 Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:10:31 +0100 Subject: [PATCH 01/10] feat(metronome): add getMetronomeVolume / saveMetronomeVolume store helpers 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) --- .../src/lib/practice-session-store.test.ts | 81 +++++++++++++++++++ .../src/lib/practice-session-store.ts | 24 ++++++ 2 files changed, 105 insertions(+) create mode 100644 frontend/next-app/src/lib/practice-session-store.test.ts diff --git a/frontend/next-app/src/lib/practice-session-store.test.ts b/frontend/next-app/src/lib/practice-session-store.test.ts new file mode 100644 index 0000000..14c2a6c --- /dev/null +++ b/frontend/next-app/src/lib/practice-session-store.test.ts @@ -0,0 +1,81 @@ +/** + * @jest-environment jsdom + */ +import { + getMetronomeVolume, + saveMetronomeVolume, +} from "./practice-session-store"; + +// jest.setup.ts replaces global.localStorage with jest.fn() stubs that don't +// actually store. Install a Map-backed replacement so round-trip tests work. +const installRealLocalStorage = () => { + const store = new Map(); + const mock: Storage = { + get length() { + return store.size; + }, + key: (i) => Array.from(store.keys())[i] ?? null, + getItem: (k) => (store.has(k) ? (store.get(k) as string) : null), + setItem: (k, v) => { + store.set(k, String(v)); + }, + removeItem: (k) => { + store.delete(k); + }, + clear: () => { + store.clear(); + }, + }; + Object.defineProperty(window, "localStorage", { + value: mock, + configurable: true, + writable: true, + }); + return store; +}; + +describe("metronome volume persistence", () => { + beforeEach(() => { + installRealLocalStorage(); + }); + + it("round-trips a value saved and then read", () => { + saveMetronomeVolume(0.6); + expect(getMetronomeVolume()).toBeCloseTo(0.6, 10); + }); + + it("returns null when nothing has been saved", () => { + expect(getMetronomeVolume()).toBeNull(); + }); + + it("clamps on save: values > 1 store as 1", () => { + saveMetronomeVolume(2); + expect(getMetronomeVolume()).toBe(1); + }); + + it("clamps on save: negative values store as 0", () => { + saveMetronomeVolume(-0.5); + expect(getMetronomeVolume()).toBe(0); + }); + + it("clamps on read: hand-edited out-of-range values are clamped", () => { + window.localStorage.setItem( + "practice:metronome-volume", + JSON.stringify({ volume: 5, updatedAt: "x" }), + ); + expect(getMetronomeVolume()).toBe(1); + }); + + it("returns null when stored JSON is corrupt", () => { + window.localStorage.setItem("practice:metronome-volume", "{ not json"); + expect(getMetronomeVolume()).toBeNull(); + }); + + it("returns null when stored object is missing the volume field", () => { + window.localStorage.setItem( + "practice:metronome-volume", + JSON.stringify({ updatedAt: "x" }), + ); + expect(getMetronomeVolume()).toBeNull(); + }); +}); diff --git a/frontend/next-app/src/lib/practice-session-store.ts b/frontend/next-app/src/lib/practice-session-store.ts index e9836d9..a557aeb 100644 --- a/frontend/next-app/src/lib/practice-session-store.ts +++ b/frontend/next-app/src/lib/practice-session-store.ts @@ -109,6 +109,30 @@ export const clearStoredRecommendation = () => { removeItem(RECOMMENDATION_KEY); }; +// --- Metronome volume (global, device-scoped) --- + +const METRONOME_VOLUME_KEY = "practice:metronome-volume"; + +export interface MetronomeVolumeRecord { + volume: number; // 0..1 + updatedAt: string; +} + +const clamp01 = (n: number): number => Math.min(1, Math.max(0, n)); + +export const getMetronomeVolume = (): number | null => { + const record = readJson(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); +}; + // --- Per-instrument project persistence (Launch Pad) --- export const INSTRUMENTS = ["Guitar", "Bass", "Drums", "Keys"] as const; From eb43aa5ac65d901324dc4bac08ca76a19cfbc4e6 Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:16:20 +0100 Subject: [PATCH 02/10] feat(metronome): route clicks through a master GainNode 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) --- .../src/lib/audio/metronome-engine.test.ts | 100 ++++++++++++++++++ .../src/lib/audio/metronome-engine.ts | 11 +- 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 frontend/next-app/src/lib/audio/metronome-engine.test.ts diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts new file mode 100644 index 0000000..c2ad51a --- /dev/null +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -0,0 +1,100 @@ +/** + * @jest-environment jsdom + */ +import { MetronomeEngine } from "./metronome-engine"; + +// Minimal Web Audio mock — enough surface for MetronomeEngine. +// Each node tracks what it's connected to so routing can be asserted. + +const makeParam = () => ({ + value: 0, + exponentialRampToValueAtTime: jest.fn(), + linearRampToValueAtTime: jest.fn(), +}); + +const makeOscillator = () => ({ + frequency: makeParam(), + connect: jest.fn(), + start: jest.fn(), + stop: jest.fn(), +}); + +const makeGain = () => ({ + gain: makeParam(), + connect: jest.fn(), + disconnect: jest.fn(), +}); + +type MockOscillator = ReturnType; +type MockGain = ReturnType; + +let createdOscillators: MockOscillator[]; +let createdGains: MockGain[]; +let mockContext: { + currentTime: number; + destination: { __tag: "destination" }; + createOscillator: jest.Mock; + createGain: jest.Mock; + close: jest.Mock; +}; + +const installAudioContextMock = () => { + createdOscillators = []; + createdGains = []; + mockContext = { + currentTime: 0, + destination: { __tag: "destination" }, + createOscillator: jest.fn(() => { + const o = makeOscillator(); + createdOscillators.push(o); + return o; + }), + createGain: jest.fn(() => { + const g = makeGain(); + createdGains.push(g); + return g; + }), + close: jest.fn(), + }; + (global as unknown as { AudioContext: unknown }).AudioContext = jest.fn( + () => mockContext, + ); +}; + +describe("MetronomeEngine — master gain routing", () => { + beforeEach(() => { + installAudioContextMock(); + }); + + it("creates a masterGain node on start() and connects it to destination", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.start(); + + // First gain created by start() is the master gain + const masterGain = createdGains[0]; + expect(masterGain).toBeDefined(); + expect(masterGain.connect).toHaveBeenCalledWith(mockContext.destination); + }); + + it("routes per-click envelope gains into masterGain, not directly to destination", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.start(); + + const masterGain = createdGains[0]; + // schedule() fires at least one click synchronously; the second gain node + // onward is a per-click envelope + const envelopeGain = createdGains[1]; + expect(envelopeGain).toBeDefined(); + expect(envelopeGain.connect).toHaveBeenCalledWith(masterGain); + expect(envelopeGain.connect).not.toHaveBeenCalledWith( + mockContext.destination, + ); + }); + + it("closes the audio context on stop()", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.start(); + engine.stop(); + expect(mockContext.close).toHaveBeenCalled(); + }); +}); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index 84da1ae..282cc50 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -10,6 +10,8 @@ export class MetronomeEngine { private nextNoteTime: number = 0; private currentBeat: number = 0; private isPlaying: boolean = false; + private masterGain: GainNode | null = null; + private masterVolume: number = 0.8; private bpm: number; private beatsPerMeasure: number; @@ -27,6 +29,9 @@ export class MetronomeEngine { start(): void { if (this.isPlaying) return; this.audioContext = new AudioContext(); + this.masterGain = this.audioContext.createGain(); + this.masterGain.gain.value = this.masterVolume * this.masterVolume; + this.masterGain.connect(this.audioContext.destination); this.isPlaying = true; this.currentBeat = 0; this.nextNoteTime = this.audioContext.currentTime; @@ -39,6 +44,10 @@ export class MetronomeEngine { clearTimeout(this.timerId); this.timerId = null; } + if (this.masterGain) { + this.masterGain.disconnect(); + this.masterGain = null; + } if (this.audioContext) { this.audioContext.close(); this.audioContext = null; @@ -78,7 +87,7 @@ export class MetronomeEngine { const gain = this.audioContext.createGain(); osc.connect(gain); - gain.connect(this.audioContext.destination); + gain.connect(this.masterGain ?? this.audioContext.destination); osc.frequency.value = isAccent ? 1000 : 800; gain.gain.value = isAccent ? 1.0 : 0.5; From 58a0f666014dc387bbf1b7ae47686192e6b9078b Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:21:54 +0100 Subject: [PATCH 03/10] test(metronome): harden engine master-gain test coverage 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) --- .../src/lib/audio/metronome-engine.test.ts | 29 +++++++++++++++++-- .../src/lib/audio/metronome-engine.ts | 4 +++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts index c2ad51a..afa1d9a 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.test.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -66,14 +66,20 @@ describe("MetronomeEngine — master gain routing", () => { installAudioContextMock(); }); - it("creates a masterGain node on start() and connects it to destination", () => { + it("creates a masterGain node FIRST on start() (before any per-click envelope) and connects it to destination exactly once", () => { const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); engine.start(); - // First gain created by start() is the master gain + // start() schedules at least one click synchronously (BPM 120, SCHEDULE_AHEAD_TIME 0.1s), + // so createdGains must contain masterGain + >=1 envelope gain. + expect(createdGains.length).toBeGreaterThanOrEqual(2); + const masterGain = createdGains[0]; expect(masterGain).toBeDefined(); expect(masterGain.connect).toHaveBeenCalledWith(mockContext.destination); + // masterGain routes to destination exactly once — guards against a regression + // where the scheduler reverts to direct-to-destination per click. + expect(masterGain.connect).toHaveBeenCalledTimes(1); }); it("routes per-click envelope gains into masterGain, not directly to destination", () => { @@ -91,10 +97,27 @@ describe("MetronomeEngine — master gain routing", () => { ); }); - it("closes the audio context on stop()", () => { + it("disconnects masterGain and closes the audio context on stop()", () => { const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); engine.start(); + const masterGain = createdGains[0]; engine.stop(); + expect(masterGain.disconnect).toHaveBeenCalled(); expect(mockContext.close).toHaveBeenCalled(); }); + + it("creates a fresh masterGain on the next start() after a stop()", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.start(); + const firstMasterGain = createdGains[0]; + engine.stop(); + + // Reinstall the mock so createdGains starts empty for the second start() + installAudioContextMock(); + engine.start(); + const secondMasterGain = createdGains[0]; + + expect(secondMasterGain).toBeDefined(); + expect(secondMasterGain).not.toBe(firstMasterGain); + }); }); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index 282cc50..eed412e 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -87,6 +87,10 @@ export class MetronomeEngine { const gain = this.audioContext.createGain(); osc.connect(gain); + // masterGain is non-null whenever isPlaying is true (start() assigns it before + // setting isPlaying, and stop() clears isPlaying before nulling it). The ?? + // fallback exists purely to satisfy the type checker, which can't prove the + // invariant through the setTimeout → schedule → playClick indirection. gain.connect(this.masterGain ?? this.audioContext.destination); osc.frequency.value = isAccent ? 1000 : 800; From cbbd9f168ccdcdf4b6e445a26ea595ee6c169834 Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:24:24 +0100 Subject: [PATCH 04/10] feat(metronome): add setVolume with square-law curve and 15 ms ramp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../src/lib/audio/metronome-engine.test.ts | 72 +++++++++++++++++++ .../src/lib/audio/metronome-engine.ts | 15 ++++ 2 files changed, 87 insertions(+) diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts index afa1d9a..4584bd0 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.test.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -121,3 +121,75 @@ describe("MetronomeEngine — master gain routing", () => { expect(secondMasterGain).not.toBe(firstMasterGain); }); }); + +describe("MetronomeEngine — setVolume", () => { + beforeEach(() => { + installAudioContextMock(); + }); + + it("applies a square-law curve on start(): masterGain initial value = volume²", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.setVolume(0.5); + engine.start(); + const masterGain = createdGains[0]; + expect(masterGain.gain.value).toBeCloseTo(0.25, 10); + }); + + it("clamps setVolume input to [0, 1]", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.setVolume(2); + engine.start(); + expect(createdGains[0].gain.value).toBeCloseTo(1, 10); + + installAudioContextMock(); + const engine2 = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine2.setVolume(-1); + engine2.start(); + expect(createdGains[0].gain.value).toBeCloseTo(0, 10); + }); + + it("coerces non-finite values (NaN, Infinity, -Infinity) to 0", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.setVolume(NaN); + engine.start(); + expect(createdGains[0].gain.value).toBe(0); + + installAudioContextMock(); + const engine2 = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine2.setVolume(Infinity); + engine2.start(); + expect(createdGains[0].gain.value).toBe(0); + + installAudioContextMock(); + const engine3 = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine3.setVolume(-Infinity); + engine3.start(); + expect(createdGains[0].gain.value).toBe(0); + }); + + it("ramps masterGain with linearRampToValueAtTime(~15 ms) when running", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.start(); + const masterGain = createdGains[0]; + mockContext.currentTime = 1.0; + + engine.setVolume(0.6); + + expect(masterGain.gain.linearRampToValueAtTime).toHaveBeenCalledTimes(1); + const [target, atTime] = ( + masterGain.gain.linearRampToValueAtTime as jest.Mock + ).mock.calls[0]; + expect(target).toBeCloseTo(0.36, 10); // 0.6² + expect(atTime).toBeCloseTo(1.015, 4); // currentTime + 0.015 + }); + + it("does NOT ramp when not running; only caches the value for next start()", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.setVolume(0.3); + // No start() yet — no masterGain exists + engine.start(); + const masterGain = createdGains[0]; + expect(masterGain.gain.linearRampToValueAtTime).not.toHaveBeenCalled(); + expect(masterGain.gain.value).toBeCloseTo(0.09, 10); // 0.3² + }); +}); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index eed412e..d98196e 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -67,6 +67,21 @@ export class MetronomeEngine { this.onBeat = callback; } + setVolume(v: number): void { + // Defend against NaN/Infinity from hand-edited localStorage or caller bugs — + // AudioParam.gain.value = NaN throws / produces undefined behavior depending on browser. + const safe = Number.isFinite(v) ? v : 0; + const clamped = Math.min(1, Math.max(0, safe)); + this.masterVolume = clamped; + if (this.audioContext && this.masterGain) { + const target = clamped * clamped; + this.masterGain.gain.linearRampToValueAtTime( + target, + this.audioContext.currentTime + 0.015, + ); + } + } + private schedule(): void { if (!this.isPlaying || !this.audioContext) return; From 46bf71568101926119e10b04ea6e4010e510562f Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:29:26 +0100 Subject: [PATCH 05/10] harden(metronome): anchor master gain ramp + name the 15 ms constant 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) --- .../src/lib/audio/metronome-engine.test.ts | 18 +++++++++++++++--- .../next-app/src/lib/audio/metronome-engine.ts | 12 ++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts index 4584bd0..c4727c4 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.test.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -8,6 +8,7 @@ import { MetronomeEngine } from "./metronome-engine"; const makeParam = () => ({ value: 0, + setValueAtTime: jest.fn(), exponentialRampToValueAtTime: jest.fn(), linearRampToValueAtTime: jest.fn(), }); @@ -167,28 +168,39 @@ describe("MetronomeEngine — setVolume", () => { expect(createdGains[0].gain.value).toBe(0); }); - it("ramps masterGain with linearRampToValueAtTime(~15 ms) when running", () => { + it("anchors the ramp with setValueAtTime(currentValue, now) before linearRampToValueAtTime(~15 ms) when running", () => { const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); engine.start(); const masterGain = createdGains[0]; + const startValue = masterGain.gain.value; // should be the default 0.8² = 0.64 mockContext.currentTime = 1.0; engine.setVolume(0.6); + // Anchor comes first: setValueAtTime with the current gain value at `now`. + expect(masterGain.gain.setValueAtTime).toHaveBeenCalledTimes(1); + const [anchorValue, anchorTime] = ( + masterGain.gain.setValueAtTime as jest.Mock + ).mock.calls[0]; + expect(anchorValue).toBeCloseTo(startValue, 10); + expect(anchorTime).toBeCloseTo(1.0, 4); + + // Then the ramp to v² at now + 15 ms. expect(masterGain.gain.linearRampToValueAtTime).toHaveBeenCalledTimes(1); const [target, atTime] = ( masterGain.gain.linearRampToValueAtTime as jest.Mock ).mock.calls[0]; expect(target).toBeCloseTo(0.36, 10); // 0.6² - expect(atTime).toBeCloseTo(1.015, 4); // currentTime + 0.015 + expect(atTime).toBeCloseTo(1.015, 4); // currentTime + MASTER_RAMP_TIME (0.015) }); - it("does NOT ramp when not running; only caches the value for next start()", () => { + it("does NOT ramp or anchor when not running; only caches the value for next start()", () => { const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); engine.setVolume(0.3); // No start() yet — no masterGain exists engine.start(); const masterGain = createdGains[0]; + expect(masterGain.gain.setValueAtTime).not.toHaveBeenCalled(); expect(masterGain.gain.linearRampToValueAtTime).not.toHaveBeenCalled(); expect(masterGain.gain.value).toBeCloseTo(0.09, 10); // 0.3² }); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index d98196e..31c0cd3 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -19,6 +19,7 @@ export class MetronomeEngine { private readonly SCHEDULE_AHEAD_TIME = 0.1; private readonly LOOKAHEAD = 25; + private readonly MASTER_RAMP_TIME = 0.015; // 15 ms — fast enough to feel instant while dragging the volume slider, slow enough to prevent audible zipper noise from direct gain.value writes. constructor(options: MetronomeOptions) { this.bpm = options.bpm; @@ -75,10 +76,13 @@ export class MetronomeEngine { this.masterVolume = clamped; if (this.audioContext && this.masterGain) { const target = clamped * clamped; - this.masterGain.gain.linearRampToValueAtTime( - target, - this.audioContext.currentTime + 0.015, - ); + const now = this.audioContext.currentTime; + // 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); } } From 6f23d4d5342661d545c60415038cf2d5399e13a3 Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:30:50 +0100 Subject: [PATCH 06/10] =?UTF-8?q?tune(metronome):=20raise=20unaccented=20b?= =?UTF-8?q?ase=20gain=200.5=20=E2=86=92=200.75?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../src/lib/audio/metronome-engine.test.ts | 25 +++++++++++++++++++ .../src/lib/audio/metronome-engine.ts | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts index c4727c4..5368688 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.test.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -205,3 +205,28 @@ describe("MetronomeEngine — setVolume", () => { expect(masterGain.gain.value).toBeCloseTo(0.09, 10); // 0.3² }); }); + +describe("MetronomeEngine — per-click base gains (accent / unaccented ratio)", () => { + beforeEach(() => { + installAudioContextMock(); + }); + + it("accented click uses base gain 1.0", () => { + const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); + engine.start(); + // createdGains[0] = masterGain, [1] = first per-click envelope. + // currentBeat starts at 0 → isAccent true on first click. + const firstClickEnvelope = createdGains[1]; + expect(firstClickEnvelope.gain.value).toBeCloseTo(1.0, 10); + }); + + it("unaccented click uses base gain 0.75 (retuned from 0.5)", () => { + // At BPM 1200 the click interval is 0.05 s. The engine's SCHEDULE_AHEAD_TIME + // is 0.1 s, so the while-loop inside schedule() queues two clicks on the + // first call: beat 0 (accented) and beat 1 (unaccented). + // createdGains[0] = masterGain, [1] = accent envelope, [2] = unaccent envelope. + const engine = new MetronomeEngine({ bpm: 1200, beatsPerMeasure: 2 }); + engine.start(); + expect(createdGains[2].gain.value).toBeCloseTo(0.75, 10); + }); +}); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index 31c0cd3..a5a4b34 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -113,7 +113,7 @@ export class MetronomeEngine { gain.connect(this.masterGain ?? this.audioContext.destination); osc.frequency.value = isAccent ? 1000 : 800; - gain.gain.value = isAccent ? 1.0 : 0.5; + gain.gain.value = isAccent ? 1.0 : 0.75; osc.start(time); gain.gain.exponentialRampToValueAtTime(0.001, time + 0.05); From ca98964cef0a43c9abe18866a1ae2f081d6cf42b Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:33:02 +0100 Subject: [PATCH 07/10] harden(metronome): document retune ratio + guard unaccent test against 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) --- frontend/next-app/src/lib/audio/metronome-engine.test.ts | 4 ++++ frontend/next-app/src/lib/audio/metronome-engine.ts | 3 +++ 2 files changed, 7 insertions(+) diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts index 5368688..c88abae 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.test.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -227,6 +227,10 @@ describe("MetronomeEngine — per-click base gains (accent / unaccented ratio)", // createdGains[0] = masterGain, [1] = accent envelope, [2] = unaccent envelope. const engine = new MetronomeEngine({ bpm: 1200, beatsPerMeasure: 2 }); engine.start(); + // Preflight: if SCHEDULE_AHEAD_TIME ever drops below the 0.05 s interval, + // only one click will schedule and createdGains[2] would be undefined. + // Fail loudly in that case rather than silently passing / crashing. + expect(createdGains.length).toBeGreaterThanOrEqual(3); expect(createdGains[2].gain.value).toBeCloseTo(0.75, 10); }); }); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index a5a4b34..62d1427 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -113,6 +113,9 @@ export class MetronomeEngine { gain.connect(this.masterGain ?? this.audioContext.destination); osc.frequency.value = isAccent ? 1000 : 800; + // Base per-click gain. Accent 1.0, unaccent 0.75 — a ~-2.5 dB gap so the + // downbeat still reads as the accent but non-downbeats aren't "too quiet + // to hear" (see issue #42). Master volume scales both via masterGain. gain.gain.value = isAccent ? 1.0 : 0.75; osc.start(time); From 3aba92e8247dee64c03e1e5946c9bcce66b99f7a Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:35:29 +0100 Subject: [PATCH 08/10] feat(metronome): add volume slider + speaker icon to MetronomeWidget 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) --- .../studio/MetronomeWidget.test.tsx | 62 +++++++++++++++++++ .../src/components/studio/MetronomeWidget.tsx | 19 ++++++ .../src/components/studio/VolumeIcon.tsx | 44 +++++++++++++ 3 files changed, 125 insertions(+) create mode 100644 frontend/next-app/src/components/studio/MetronomeWidget.test.tsx create mode 100644 frontend/next-app/src/components/studio/VolumeIcon.tsx diff --git a/frontend/next-app/src/components/studio/MetronomeWidget.test.tsx b/frontend/next-app/src/components/studio/MetronomeWidget.test.tsx new file mode 100644 index 0000000..537776e --- /dev/null +++ b/frontend/next-app/src/components/studio/MetronomeWidget.test.tsx @@ -0,0 +1,62 @@ +/** + * @jest-environment jsdom + */ +import { render, screen, fireEvent } from "@testing-library/react"; +import MetronomeWidget from "./MetronomeWidget"; + +// Props mirror the real MetronomeWidgetProps interface. If the interface +// has diverged by the time this runs, read the widget and reconcile — the +// point of this test is the volume slider, not the rest. +const baseProps = { + bpm: 120, + isActive: false, + currentBeat: 0, + beatsPerMeasure: 4, + onBpmChange: jest.fn(), + onBeatsPerMeasureChange: jest.fn(), + onToggle: jest.fn(), + onTapTempo: jest.fn(), + volume: 0.8, + onVolumeChange: jest.fn(), +}; + +describe("MetronomeWidget — volume slider", () => { + it("renders a slider with aria-label 'Metronome volume' and the supplied value", () => { + render(); + const slider = screen.getByLabelText("Metronome volume") as HTMLInputElement; + expect(slider).toBeInTheDocument(); + expect(slider.type).toBe("range"); + expect(slider.min).toBe("0"); + expect(slider.max).toBe("1"); + expect(Number(slider.value)).toBeCloseTo(0.3, 2); + }); + + it("calls onVolumeChange with the new numeric value when dragged", () => { + const onVolumeChange = jest.fn(); + render( + , + ); + const slider = screen.getByLabelText("Metronome volume"); + fireEvent.change(slider, { target: { value: "0.72" } }); + expect(onVolumeChange).toHaveBeenCalledWith(0.72); + }); + + it("shows the 'muted' icon when volume is 0", () => { + render(); + expect(screen.getByTestId("volume-icon-muted")).toBeInTheDocument(); + }); + + it("shows the 'low' icon when 0 < volume < 0.5", () => { + render(); + expect(screen.getByTestId("volume-icon-low")).toBeInTheDocument(); + }); + + it("shows the 'high' icon when volume >= 0.5", () => { + render(); + expect(screen.getByTestId("volume-icon-high")).toBeInTheDocument(); + }); +}); diff --git a/frontend/next-app/src/components/studio/MetronomeWidget.tsx b/frontend/next-app/src/components/studio/MetronomeWidget.tsx index c5cb30e..741485c 100644 --- a/frontend/next-app/src/components/studio/MetronomeWidget.tsx +++ b/frontend/next-app/src/components/studio/MetronomeWidget.tsx @@ -4,6 +4,7 @@ import React from "react"; import { Button } from "@/components/ui/button"; import { Minus, Plus } from "@phosphor-icons/react"; import { MotionDiv } from "@/components/ui/motion-wrapper"; +import { VolumeIcon } from "./VolumeIcon"; const TIME_SIGNATURES = [ { label: "2/4", beats: 2 }, @@ -23,6 +24,8 @@ interface MetronomeWidgetProps { onBeatsPerMeasureChange: (beats: number) => void; onToggle: () => void; onTapTempo: () => void; + volume: number; + onVolumeChange: (volume: number) => void; } export default function MetronomeWidget({ @@ -34,6 +37,8 @@ export default function MetronomeWidget({ onBeatsPerMeasureChange, onToggle, onTapTempo, + volume, + onVolumeChange, }: MetronomeWidgetProps) { const handleBpmChange = (value: number) => { onBpmChange(Math.max(20, Math.min(300, value))); @@ -79,6 +84,20 @@ export default function MetronomeWidget({ +
+ + onVolumeChange(Number(e.target.value))} + className="flex-1 h-1.5 bg-muted rounded-full appearance-none cursor-pointer accent-primary" + aria-label="Metronome volume" + /> +
+
{Array.from({ length: beatsPerMeasure }).map((_, i) => { const isActiveBeat = currentBeat === i; diff --git a/frontend/next-app/src/components/studio/VolumeIcon.tsx b/frontend/next-app/src/components/studio/VolumeIcon.tsx new file mode 100644 index 0000000..1e247ed --- /dev/null +++ b/frontend/next-app/src/components/studio/VolumeIcon.tsx @@ -0,0 +1,44 @@ +"use client"; + +import { SpeakerHigh, SpeakerLow, SpeakerX } from "@phosphor-icons/react"; + +export interface VolumeIconProps { + volume: number; // 0..1 + size?: number; + className?: string; +} + +export function VolumeIcon({ + volume, + size = 16, + className, +}: VolumeIconProps) { + if (volume === 0) { + return ( + + ); + } + if (volume < 0.5) { + return ( + + ); + } + return ( + + ); +} From 92ce47860062c3a2b8aff85e58ffa013dea3a207 Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Fri, 17 Apr 2026 23:41:06 +0100 Subject: [PATCH 09/10] feat(metronome): wire volume state into practice-timer page 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) --- .../practice-timer/__tests__/page.test.tsx | 3 +++ .../next-app/src/app/practice-timer/page.tsx | 23 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/frontend/next-app/src/app/practice-timer/__tests__/page.test.tsx b/frontend/next-app/src/app/practice-timer/__tests__/page.test.tsx index abdc68b..f11890a 100644 --- a/frontend/next-app/src/app/practice-timer/__tests__/page.test.tsx +++ b/frontend/next-app/src/app/practice-timer/__tests__/page.test.tsx @@ -30,6 +30,7 @@ jest.mock('@/lib/audio/metronome-engine', () => ({ setBpm: jest.fn(), setBeatsPerMeasure: jest.fn(), setOnBeat: jest.fn(), + setVolume: jest.fn(), })), })); @@ -53,6 +54,8 @@ jest.mock('@/lib/practice-session-store', () => ({ clearStoredSessionSnapshot: jest.fn(), getProject: (...args: unknown[]) => mockGetProject(...args), saveProject: (...args: unknown[]) => mockSaveProject(...args), + getMetronomeVolume: jest.fn().mockReturnValue(null), + saveMetronomeVolume: jest.fn(), INSTRUMENTS: ['Guitar', 'Bass', 'Drums', 'Keys'] as const, })); diff --git a/frontend/next-app/src/app/practice-timer/page.tsx b/frontend/next-app/src/app/practice-timer/page.tsx index 6f68861..7a1763f 100644 --- a/frontend/next-app/src/app/practice-timer/page.tsx +++ b/frontend/next-app/src/app/practice-timer/page.tsx @@ -22,7 +22,9 @@ import { AnimatePresence } from "framer-motion"; import { clearStoredPracticeSetup, clearStoredSessionSnapshot, + getMetronomeVolume, getStoredPracticeSetup, + saveMetronomeVolume, saveStoredPracticeSetup, saveStoredSessionSnapshot, getProject, @@ -77,6 +79,18 @@ function PracticeTimerContent() { const [currentBeat, setCurrentBeat] = useState(-1); const [beatsPerMeasure, setBeatsPerMeasure] = useState(4); const [, setTapTimes] = useState([]); + const [metronomeVolume, setMetronomeVolumeState] = useState(0.8); + + // Hydrate from localStorage after mount (SSR-safe) + useEffect(() => { + const saved = getMetronomeVolume(); + if (saved !== null) setMetronomeVolumeState(saved); + }, []); + + const handleMetronomeVolumeChange = useCallback((v: number) => { + setMetronomeVolumeState(v); + saveMetronomeVolume(v); + }, []); // ─── Tuner state ───────────────────────────────────────────────────── const [tunerActive, setTunerActive] = useState(false); @@ -617,10 +631,11 @@ function PracticeTimerContent() { beatsPerMeasure, onBeat: handleBeatCallback, }); + engine.setVolume(metronomeVolume); metronomeRef.current = engine; engine.start(); setMetronomeActive(true); - }, [bpm, beatsPerMeasure, handleBeatCallback]); + }, [bpm, beatsPerMeasure, handleBeatCallback, metronomeVolume]); const handleMetronomeStop = useCallback(() => { metronomeRef.current?.stop(); @@ -641,6 +656,10 @@ function PracticeTimerContent() { metronomeRef.current?.setOnBeat(handleBeatCallback); }, [handleBeatCallback]); + useEffect(() => { + metronomeRef.current?.setVolume(metronomeVolume); + }, [metronomeVolume]); + const handleBpmChange = (value: number) => { const clamped = Math.max(20, Math.min(300, value)); setBpm(clamped); @@ -1001,6 +1020,8 @@ function PracticeTimerContent() { onBeatsPerMeasureChange={setBeatsPerMeasure} onToggle={handleMetronomeToggle} onTapTempo={handleTapTempo} + volume={metronomeVolume} + onVolumeChange={handleMetronomeVolumeChange} />
From 746e1a11f17f11d4dc16b330e00ecd7770ec263e Mon Sep 17 00:00:00 2001 From: Daniel Adekugbe Date: Sat, 18 Apr 2026 00:38:04 +0100 Subject: [PATCH 10/10] harden(metronome): add 3 ms attack ramp to kill first-click pop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../src/lib/audio/metronome-engine.test.ts | 33 ++++++++++++++++--- .../src/lib/audio/metronome-engine.ts | 12 +++++-- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/frontend/next-app/src/lib/audio/metronome-engine.test.ts b/frontend/next-app/src/lib/audio/metronome-engine.test.ts index c88abae..c35d933 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.test.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.test.ts @@ -206,21 +206,42 @@ describe("MetronomeEngine — setVolume", () => { }); }); -describe("MetronomeEngine — per-click base gains (accent / unaccented ratio)", () => { +describe("MetronomeEngine — per-click attack envelope (accent / unaccented ratio)", () => { beforeEach(() => { installAudioContextMock(); }); - it("accented click uses base gain 1.0", () => { + it("accented click ramps 0 → 1.0 over CLICK_ATTACK_TIME (3 ms) before oscillator.start", () => { const engine = new MetronomeEngine({ bpm: 120, beatsPerMeasure: 4 }); engine.start(); // createdGains[0] = masterGain, [1] = first per-click envelope. // currentBeat starts at 0 → isAccent true on first click. const firstClickEnvelope = createdGains[1]; - expect(firstClickEnvelope.gain.value).toBeCloseTo(1.0, 10); + + // Attack starts at 0 at the click time, then ramps linearly to peak 1.0 + // at time + 0.003 s. + expect(firstClickEnvelope.gain.setValueAtTime).toHaveBeenCalledWith(0, expect.any(Number)); + const [, rampValue, rampTime] = [ + null, + ...(firstClickEnvelope.gain.linearRampToValueAtTime as jest.Mock).mock.calls[0], + ]; + expect(rampValue).toBeCloseTo(1.0, 10); + + const setValueArgs = (firstClickEnvelope.gain.setValueAtTime as jest.Mock).mock.calls[0]; + const startTime = setValueArgs[1]; + expect(rampTime).toBeCloseTo(startTime + 0.003, 6); + + // And the attack ramp is scheduled BEFORE the oscillator starts — this is + // the anti-pop invariant. Jest tracks invocation order across all mocks. + const firstOsc = createdOscillators[0]; + const setValueOrder = (firstClickEnvelope.gain.setValueAtTime as jest.Mock).mock.invocationCallOrder[0]; + const linearOrder = (firstClickEnvelope.gain.linearRampToValueAtTime as jest.Mock).mock.invocationCallOrder[0]; + const oscStartOrder = (firstOsc.start as jest.Mock).mock.invocationCallOrder[0]; + expect(setValueOrder).toBeLessThan(oscStartOrder); + expect(linearOrder).toBeLessThan(oscStartOrder); }); - it("unaccented click uses base gain 0.75 (retuned from 0.5)", () => { + it("unaccented click ramps 0 → 0.75 (retuned from 0.5) over CLICK_ATTACK_TIME", () => { // At BPM 1200 the click interval is 0.05 s. The engine's SCHEDULE_AHEAD_TIME // is 0.1 s, so the while-loop inside schedule() queues two clicks on the // first call: beat 0 (accented) and beat 1 (unaccented). @@ -231,6 +252,8 @@ describe("MetronomeEngine — per-click base gains (accent / unaccented ratio)", // only one click will schedule and createdGains[2] would be undefined. // Fail loudly in that case rather than silently passing / crashing. expect(createdGains.length).toBeGreaterThanOrEqual(3); - expect(createdGains[2].gain.value).toBeCloseTo(0.75, 10); + const unaccentEnvelope = createdGains[2]; + const rampCall = (unaccentEnvelope.gain.linearRampToValueAtTime as jest.Mock).mock.calls[0]; + expect(rampCall[0]).toBeCloseTo(0.75, 10); }); }); diff --git a/frontend/next-app/src/lib/audio/metronome-engine.ts b/frontend/next-app/src/lib/audio/metronome-engine.ts index 62d1427..ac775d2 100644 --- a/frontend/next-app/src/lib/audio/metronome-engine.ts +++ b/frontend/next-app/src/lib/audio/metronome-engine.ts @@ -20,6 +20,7 @@ export class MetronomeEngine { private readonly SCHEDULE_AHEAD_TIME = 0.1; private readonly LOOKAHEAD = 25; private readonly MASTER_RAMP_TIME = 0.015; // 15 ms — fast enough to feel instant while dragging the volume slider, slow enough to prevent audible zipper noise from direct gain.value writes. + private readonly CLICK_ATTACK_TIME = 0.003; // 3 ms — per-click attack ramp. Imperceptibly short as a volume change, but long enough to kill the DC-offset pop that hard-stepping from 0 to peak produces at osc.start(time). constructor(options: MetronomeOptions) { this.bpm = options.bpm; @@ -116,10 +117,17 @@ export class MetronomeEngine { // Base per-click gain. Accent 1.0, unaccent 0.75 — a ~-2.5 dB gap so the // downbeat still reads as the accent but non-downbeats aren't "too quiet // to hear" (see issue #42). Master volume scales both via masterGain. - gain.gain.value = isAccent ? 1.0 : 0.75; + const peak = isAccent ? 1.0 : 0.75; + + // Attack: ramp 0 → peak over CLICK_ATTACK_TIME (3 ms). Scheduled BEFORE + // osc.start so the oscillator's first sample lands inside a smooth envelope + // instead of a hard step — eliminates the DC-offset click/pop artifact. + gain.gain.setValueAtTime(0, time); + gain.gain.linearRampToValueAtTime(peak, time + this.CLICK_ATTACK_TIME); + // Decay: exponential down to ~silence over 50 ms total click length. + gain.gain.exponentialRampToValueAtTime(0.001, time + 0.05); osc.start(time); - gain.gain.exponentialRampToValueAtTime(0.001, time + 0.05); osc.stop(time + 0.05); } }