refactor: add unified InstrumentEngine interface and factory

[ChuxiJ]

Summary

• Creates InstrumentEngine interface with noteOn/noteOff/triggerAttackRelease/setParameter/releaseAll/removeTrack/dispose
• Adds InstrumentFactory with adapter pattern wrapping existing engines (SynthEngine, SamplerEngine, FM stub)
• Deprecates legacy SynthEngine.createSynthForPreset() via @deprecated JSDoc
• Adds setParameter stub to SamplerEngine
• 7 unit tests for factory routing logic

Test plan

• npx tsc --noEmit — 0 type errors
• npm test — 2983 tests pass (7 new)
• npm run build — succeeds

Closes #1031

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Introduces a unified InstrumentEngine abstraction and an InstrumentFactory to select an engine implementation based on TrackInstrument.kind, providing adapters over existing SynthEngine/SamplerEngine plus an FM stub.

Changes:

• Added InstrumentEngine interface (note on/off, attack-release, parameter setting, teardown).
• Added InstrumentFactory with per-kind singleton adapters for subtractive, sampler, and FM fallback routing.
• Deprecated legacy synth preset creation API and added a no-op setParameter stub to SamplerEngine, plus new unit tests for factory routing.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/engine/InstrumentEngine.ts	Defines the new unified engine interface.
src/engine/InstrumentFactory.ts	Implements factory + adapters wrapping legacy engines.
src/engine/SynthEngine.ts	Adds @deprecated JSDoc to legacy preset creation / engine class.
src/engine/SamplerEngine.ts	Adds a stub setParameter method for interface alignment.
src/engine/tests/InstrumentFactory.test.ts	Adds tests validating factory routing and adapter method presence.

---

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

[Copilot]

InstrumentEngine doesn’t specify the expected velocity range (MIDI 0–127 vs normalized 0–1), but existing engines use both conventions (SynthEngine.noteOn treats velocity as 0–127, while sampler playback often uses 0–1). Please document a single convention in this interface and ensure all adapters/engines convert to it consistently to avoid very quiet notes or extremely loud sampler playback.

Suggested change

	*/
	export interface InstrumentEngine {
	/** Trigger note-on for a track (for live playing / recording). */
	noteOn(trackId: string, pitch: number, velocity: number): void;
	/** Trigger note-off for a track. */
	noteOff(trackId: string, pitch: number): void;
	/** Play a note with a fixed duration (for sequenced playback). */
	*
	* Velocity convention:
	* - All methods on this interface that take a `velocity` argument expect a
	* normalized value in the range [0, 1], where 0 is silent and 1 is the
	* maximum playing intensity supported by the engine.
	* - Adapters or callers that receive MIDI velocities in the range [0, 127]
	* must convert them to this normalized range before calling into an
	* `InstrumentEngine` implementation (e.g. `normalized = midi / 127`).
	* - Individual engines are free to internally map this normalized value to
	* whatever gain/velocity model they use, but they must treat the input as
	* already normalized.
	*/
	export interface InstrumentEngine {
	/**
	* Trigger note-on for a track (for live playing / recording).
	*
	* @param trackId Identifier of the track the note belongs to.
	* @param pitch Pitch of the note (MIDI note number or engine-specific).
	* @param velocity Normalized velocity in the range [0, 1], where 0 is
	* silent and 1 is maximum intensity. Callers passing MIDI
	* velocities [0, 127] must convert to this range first.
	*/
	noteOn(trackId: string, pitch: number, velocity: number): void;
	/** Trigger note-off for a track. */
	noteOff(trackId: string, pitch: number): void;
	/**
	* Play a note with a fixed duration (for sequenced playback).
	*
	* @param trackId Identifier of the track the note belongs to.
	* @param pitch Pitch of the note (MIDI note number or engine-specific).
	* @param duration Duration of the note in seconds (or engine-specific units).
	* @param velocity Normalized velocity in the range [0, 1], where 0 is
	* silent and 1 is maximum intensity. Adapters using MIDI
	* velocities [0, 127] must normalize before calling this.
	*/

[Copilot]

SynthEngineAdapter.triggerAttackRelease hardcodes the preset to 'piano', which means sequenced playback will ignore the track’s actual SubtractiveTrackInstrument.preset (and can also overwrite an already-configured synth for the track via ensureTrackSynth). Consider deriving the preset from per-track configuration (e.g., via setParameter('preset', ...)), or avoid calling playNote (which requires a preset) and instead trigger on the already-ensured synth instance for the track.

Suggested change

	// Delegate to the singleton which handles MIDI-to-freq conversion and preset routing.
	void synthEngine.playNote(trackId, pitch, velocity, duration, 'piano');
	// Use the already-configured synth for this track instead of forcing a specific preset.
	this.noteOn(trackId, pitch, velocity);
	setTimeout(() => {
	this.noteOff(trackId, pitch);
	}, duration);

[Copilot]

SamplerEngineAdapter.triggerAttackRelease forwards velocity directly, but SamplerEngine.triggerAttackRelease appears to expect a normalized 0–1 velocity (call sites clamp note.velocity to 0–1, and noteOn converts MIDI 0–127 to 0–1). If a consumer uses MIDI velocity (0–127) like SynthEngineAdapter does, this will drive the sampler gain envelope far above 1. Please standardize the velocity scale for InstrumentEngine and convert here accordingly.

Suggested change

	samplerEngine.triggerAttackRelease(trackId, pitch, duration, velocity);
	// InstrumentEngine uses MIDI velocity (0–127); SamplerEngine.triggerAttackRelease expects 0–1.
	const clampedVelocity = Math.max(0, Math.min(127, velocity));
	const normalizedVelocity = clampedVelocity / 127;
	samplerEngine.triggerAttackRelease(trackId, pitch, duration, normalizedVelocity);

[Copilot]

SamplerEngineAdapter.setParameter is a no-op even though SamplerEngine now defines a setParameter method. Delegating to samplerEngine.setParameter(...) would keep parameter-handling behavior in one place and prevents the adapter and engine from drifting as real-time automation gets implemented.

Suggested change

	setParameter(_trackId: string, _name: string, _value: number | string | boolean): void {
	// Sampler parameters (ADSR, root note, etc.) are set via ensureTrackSampler config.
	// A future PR will wire individual param updates here.
	setParameter(trackId: string, name: string, value: number | string | boolean): void {
	samplerEngine.setParameter(trackId, name, value);

[Copilot]

The FM adapter’s doc says kind: 'fm' tracks fall back to the subtractive synth using FmTrackInstrument.fallbackPreset, but the implementation never receives or applies fallbackPreset (it always delegates to the subtractive adapter behavior). Either remove/adjust this documentation or add a mechanism to pass/apply fallbackPreset (e.g., via a standardized setParameter name).

Suggested change

	* `kind: 'fm'` fall back to the subtractive synth using {@link FmTrackInstrument.fallbackPreset}.
	* Once a dedicated FM engine exists this adapter will delegate to it.
	* `kind: 'fm'` currently fall back to the subtractive synth via
	* {@link SynthEngineAdapter}. FM-specific parameters (including any
	* fallback preset) are not yet applied here. Once a dedicated FM engine
	* exists this adapter will delegate to it.

[Copilot]

FmEngineAdapter creates a new SynthEngineAdapter instance for fallback even though the factory already maintains a singleton subtractiveAdapter. Reusing the existing subtractive adapter avoids duplicated wrapper instances and reduces the risk of divergent behavior if the adapter later gains per-track state/configuration.

[ChuxiJ]

⚠️ Merge conflict — conflicts with main after recent merges. CI was green on the original branch. Needs rebase to resolve conflicts, likely in projectStore.ts after the MIDI slice extraction (#1046).

---

Generated by Claude Code