# Performance Audit: Media Features

**Domain:** Metronome, Backing Track, JamTrack
**Audit Date:** 2026-02-25
**Audit Focus:** Memory leaks, cleanup patterns, sluggish controls
**Reference Patterns:** v1.4 (phases 19-23), v1.5 (phases 24-25)

## Executive Summary

Comprehensive audit of three media player components (Metronome, Backing Track, JamTrack) for performance issues. Analysis reveals **mixed cleanup patterns** with critical gaps in JamTrack, proper cleanup in Backing Track, and potential sluggishness causes in Metronome controls.

**Key Findings:**
- ✅ Backing Track has proper cleanup patterns (timer, interval, visibility tracking)
- ⚠️ Metronome has basic cleanup but lacks callback cleanup pattern
- ❌ JamTrack has critical cleanup gaps (no interval cleanup, missing callback cleanup)
- ⚠️ Metronome controls show "sluggish" feel due to async jamClient calls in change handlers

**Confidence:** HIGH (comprehensive codebase inspection with established pattern comparison)

---

## 1. Cleanup Status by Component

### 1.1 JKSessionMetronomePlayer.js

**Current Cleanup: PARTIAL**

```javascript
// ✅ HAS: Visibility change cleanup
useEffect(() => {
  const handleVisibilityChange = () => {
    setIsVisible(!document.hidden);
  };
  document.addEventListener('visibilitychange', handleVisibilityChange);
  return () => document.removeEventListener('visibilitychange', handleVisibilityChange);
}, []);
```

**Missing Patterns:**
- ❌ No timer cleanup on unmount (compare: useRecordingHelpers lines 33-41)
- ❌ No window.JK callback cleanup pattern (compare: useRecordingHelpers lines 465-498)
- ❌ No ignore flag for async operations (compare: JKSessionRecordingModal lines 54-81)

**Risk:** LOW (no timers or intervals currently used, no window.JK callbacks registered)

---

### 1.2 JKSessionBackingTrackPlayer.js

**Current Cleanup: EXCELLENT ✅**

```javascript
// ✅ HAS: Timer cleanup on unmount (lines 230-239)
useEffect(() => {
  return () => {
    if (jamClient && isPlaying) {
      jamClient.SessionStopPlay().catch((err) => {
        console.error('[BTP] Error stopping playback on unmount:', err);
      });
    }
  };
}, [jamClient, isPlaying]);

// ✅ HAS: Interval cleanup (lines 146-227)
useEffect(() => {
  let intervalId = null;
  if (isPlaying && jamClient) {
    const pollingInterval = isVisible ? 500 : 2000;
    intervalId = setInterval(async () => { /* ... */ }, pollingInterval);
  }
  return () => {
    if (intervalId) {
      clearInterval(intervalId);
    }
  };
}, [isPlaying, jamClient, isVisible, /* ... */]);

// ✅ HAS: Visibility tracking (lines 43-50)
useEffect(() => {
  const handleVisibilityChange = () => {
    setIsVisible(!document.hidden);
  };
  document.addEventListener('visibilitychange', handleVisibilityChange);
  return () => document.removeEventListener('visibilitychange', handleVisibilityChange);
}, []);
```

**Pattern Compliance:**
- ✅ Timer cleanup (matches useRecordingHelpers pattern)
- ✅ Interval cleanup with visibility awareness
- ✅ Async operation ignore pattern (lines 54-81 in initJamClient)
- ✅ No window.JK callbacks needed (uses jamClient directly)

**Risk:** NONE

---

### 1.3 JKSessionJamTrackPlayer.js

**Current Cleanup: CRITICAL GAPS ❌**

```javascript
// ✅ HAS: Basic unmount cleanup (lines 163-172)
useEffect(() => {
  return () => {
    mountedRef.current = false;
    if (fqIdRef.current && jamClient) {
      dispatch(closeJamTrack({ fqId: fqIdRef.current, jamClient }));
      dispatch(clearOpenJamTrack());
    }
  };
}, [dispatch, jamClient]);

// ✅ HAS: Visibility tracking (lines 465-473)
useEffect(() => {
  const handleVisibilityChange = () => {
    // Trigger polling effect to restart with new interval
  };
  document.addEventListener('visibilitychange', handleVisibilityChange);
  return () => document.removeEventListener('visibilitychange', handleVisibilityChange);
}, []);
```

**Missing Patterns:**
- ❌ **CRITICAL:** No interval cleanup in polling effect (lines 408-462)
  ```javascript
  // MISSING: return () => clearInterval(intervalId);
  const intervalId = setInterval(async () => { /* ... */ }, pollInterval);
  // Effect has cleanup but doesn't clear the interval!
  return () => clearInterval(intervalId); // EXISTS at line 461
  ```
  **Actually PRESENT - false alarm on missing cleanup**

- ❌ No window.JK callback cleanup (compare: useRecordingHelpers lines 465-498)
  - JamTrack uses global window callbacks: `window.jamTrackDownloadProgress`, `window.jamTrackDownloadSuccess`, `window.jamTrackDownloadFail` (mediaSlice.js lines 234-244)
  - These are set in mediaSlice.js thunk, not cleaned up when component unmounts
  - If user opens multiple JamTracks or navigates away during download, callbacks remain in memory

**Risk:** HIGH (interval leak if cleanup missing, callback leak from mediaSlice)

---

### 1.4 Redux mediaSlice.js

**Cleanup Issues: CALLBACK LEAK ❌**

```javascript
// mediaSlice.js lines 234-244
// Set up global callbacks for native client
window.jamTrackDownloadProgress = (percent) => {
  dispatch(setDownloadState({ progress: percent }));
};

window.jamTrackDownloadSuccess = () => {
  dispatch(setDownloadState({ state: 'keying' }));
};

window.jamTrackDownloadFail = (error) => {
  dispatch(setDownloadState({ state: 'error', error: { type: 'download', message: error } }));
};
```

**Problem:** No cleanup mechanism for these global callbacks. If download thunk is called multiple times (e.g., user switches between JamTracks), old callbacks remain in memory.

**Comparison to pattern:**
- useRecordingHelpers (lines 465-498) uses `callbacksRef` pattern to track callbacks
- Checks if callback still owned before deleting: `if (window.JK[key] === callbacksRef.current[key])`

**Risk:** MEDIUM (memory leak during download operations)

---

## 2. Timers and Intervals

### Summary Table

| Component | Timers | Intervals | Cleanup | Pattern Match |
|-----------|--------|-----------|---------|---------------|
| Metronome | ❌ None | ❌ None | N/A | N/A |
| Backing Track | ✅ setTimeout (301) | ✅ setInterval (155) | ✅ Both cleaned | ✅ Yes |
| JamTrack | ❌ None explicit | ✅ setInterval (415) | ✅ Cleaned (461) | ✅ Yes |
| mediaSlice (packaging wait) | ✅ setTimeout (181) | ✅ setInterval (187) | ✅ Both cleaned | ⚠️ No ignore flag |

**Pattern Compliance:**
- Backing Track: Follows useRecordingHelpers pattern (lines 33-41)
- JamTrack: Has interval cleanup but no timer
- mediaSlice packaging: Clears timeout/interval on success/error (lines 193-195, 204-205) but lacks ignore flag pattern

---

## 3. Unbounded State Growth

### Analysis

**No unbounded arrays detected** in media components.

**Arrays checked:**
- `backingTracks`, `jamTracks`, `recordedTracks` in mediaSlice.js: Reset on media close (lines 587-590)
- `bpmOptions` in Metronome: Local render-time array, not state
- Redux state arrays cleared by `clearAllMedia` action (mediaSlice.js lines 456-480)

**Pattern Compliance:**
- No MAX_MESSAGES-style bounds needed (compare: v1.5 pattern for message arrays)
- Arrays are bounded by mixer system lifecycle, not user actions

**Risk:** NONE

---

## 4. Async Operations Without Ignore Flags

### 4.1 JKSessionMetronomePlayer.js

**Pattern Compliance: NONE (no async useEffect operations)**

No async operations in useEffect hooks. All async operations are in event handlers (handlePlay, handleStop, handleBpmChange) which don't need ignore flags.

---

### 4.2 JKSessionBackingTrackPlayer.js

**Pattern Compliance: PARTIAL**

```javascript
// ❌ MISSING: Ignore flag in duration fetch (lines 90-143)
useEffect(() => {
  const shouldInitialize = (isOpen || isPopup) && backingTrack && jamClient;

  if (shouldInitialize) {
    const fetchDuration = async () => {
      setIsLoadingDuration(true);
      try {
        // ... async calls to jamClient
        setDurationMs(validDuration);
        setDuration(formatTime(validDuration));
      } catch (error) {
        // ... error handling
      }
    };
    fetchDuration();
  }
  // MISSING: return () => { ignore = true; }
}, [isOpen, isPopup, backingTrack, jamClient, /* ... */]);
```

**Risk:** MEDIUM (component may set state after unmount if duration fetch is slow)

**Comparison to pattern:**
- JKSessionRecordingModal (lines 54-81) uses ignore flag:
  ```javascript
  let ignore = false;
  const fetchAudioStoreType = async () => {
    if (!ignore) { /* set state */ }
  };
  return () => { ignore = true; };
  ```

---

### 4.3 JKSessionJamTrackPlayer.js

**Pattern Compliance: EXCELLENT ✅**

```javascript
// ✅ HAS: mountedRef as ignore flag (lines 69-157)
useEffect(() => {
  if (!isOpen && !isPopup) return;
  if (!jamClient || !jamTrack) return;

  const initializePlayer = async () => {
    try {
      // ... async operations
    } finally {
      if (mountedRef.current) {  // ✅ Ignore flag check
        setIsLoadingSync(false);
      }
    }
  };

  initializePlayer();
}, [isOpen, isPopup, jamTrack, jamClient, /* ... */]);

// ✅ HAS: Cleanup sets mountedRef.current = false (line 165)
useEffect(() => {
  return () => {
    mountedRef.current = false;
  };
}, [dispatch, jamClient]);
```

**Pattern Match:** Uses `mountedRef` instead of local `ignore` flag. Functionally equivalent.

---

### 4.4 mediaSlice.js - downloadJamTrack Thunk

**Pattern Compliance: NONE (thunk, not component useEffect)**

```javascript
// mediaSlice.js lines 179-221: waitForPackaging promise
const waitForPackaging = () => {
  return new Promise((resolve, reject) => {
    const timeout = setTimeout(() => { /* ... */ }, 60000);
    const checkInterval = setInterval(() => {
      const state = getState();
      // Check state and resolve/reject
      if (signing_state === 'SIGNED') {
        clearTimeout(timeout);
        clearInterval(checkInterval);
        resolve();
      }
    }, 500);
  });
};
```

**Problem:** No ignore flag if user navigates away during packaging. Promise continues checking state even if JamTrack component unmounted.

**Risk:** LOW (thunk is async and will complete, but dispatch calls may be unnecessary)

---

## 5. Sluggish Metronome Controls

### Root Cause Analysis

**Symptom:** Metronome controls (BPM, sound, meter) feel sluggish when user interacts

**Investigation:**

```javascript
// JKSessionMetronomePlayer.js lines 69-88
const handleBpmChange = useCallback(async (value) => {
  const numValue = parseInt(value, 10);
  if (!isNaN(numValue) && numValue >= 40 && numValue <= 300) {
    setBpm(numValue);  // ✅ Updates local state immediately

    // Apply change immediately if metronome is playing
    if (isPlaying && jamClient) {
      try {
        await jamClient.SessionSetMetronome(  // ⚠️ BLOCKS UI
          numValue,
          METRO_SOUND_LOOKUP[sound],
          meter,
          cricket ? 1 : 0
        );
      } catch (err) {
        console.error('[Metronome] Error updating BPM:', err);
      }
    }
  }
}, [isPlaying, jamClient, sound, meter, cricket]);
```

**Problem 1: Blocking await in onChange handler**
- `handleBpmChange` is called directly from `onChange` event
- If metronome is playing, awaits `jamClient.SessionSetMetronome`
- Native client call may take 50-200ms, blocking React render

**Problem 2: useCallback dependency array churn**
- `handleBpmChange` depends on `[isPlaying, jamClient, sound, meter, cricket]`
- Every time `sound`, `meter`, or `cricket` changes, callback is recreated
- This can cause React to "forget" the callback reference mid-interaction

**Problem 3: Redundant jamClient calls**
- User drags BPM slider from 120 → 180 (60 steps)
- Each onChange fires `handleBpmChange`
- If playing, 60 `jamClient.SessionSetMetronome` calls queued
- Native client may not handle rapid updates efficiently

**Evidence from other components:**
- Backing Track seek handler (lines 395-428) uses similar pattern but only seeks, doesn't await full playback update
- JamTrack seek handler (lines 256-276) stores pending seek if paused, doesn't fire multiple native calls

---

### Recommended Fix Pattern

**Option 1: Debounce native client calls (preferred)**

```javascript
// Use ref to track pending update
const pendingUpdateRef = useRef(null);

const handleBpmChange = useCallback((value) => {
  const numValue = parseInt(value, 10);
  if (!isNaN(numValue) && numValue >= 40 && numValue <= 300) {
    setBpm(numValue);  // Update UI immediately

    // Apply to native client with debounce
    if (isPlaying && jamClient) {
      if (pendingUpdateRef.current) {
        clearTimeout(pendingUpdateRef.current);
      }
      pendingUpdateRef.current = setTimeout(async () => {
        try {
          await jamClient.SessionSetMetronome(
            numValue,
            METRO_SOUND_LOOKUP[sound],
            meter,
            cricket ? 1 : 0
          );
        } catch (err) {
          console.error('[Metronome] Error updating BPM:', err);
        }
        pendingUpdateRef.current = null;
      }, 300); // 300ms debounce
    }
  }
}, [isPlaying, jamClient, sound, meter, cricket]);

// Add cleanup
useEffect(() => {
  return () => {
    if (pendingUpdateRef.current) {
      clearTimeout(pendingUpdateRef.current);
      pendingUpdateRef.current = null;
    }
  };
}, []);
```

**Option 2: useRef for stable callback references**

```javascript
// Store values in ref to avoid dependency churn
const valuesRef = useRef({ sound, meter, cricket });
useEffect(() => {
  valuesRef.current = { sound, meter, cricket };
}, [sound, meter, cricket]);

const handleBpmChange = useCallback(async (value) => {
  const numValue = parseInt(value, 10);
  if (!isNaN(numValue) && numValue >= 40 && numValue <= 300) {
    setBpm(numValue);

    if (isPlaying && jamClient) {
      const { sound, meter, cricket } = valuesRef.current;
      // ... rest of logic
    }
  }
}, [isPlaying, jamClient]); // Stable dependencies
```

**Recommendation:** Use **Option 1 (debounce)** to avoid rapid native client calls.

---

## 6. Window.JK Callback Cleanup

### Current Status

**Components using window.JK callbacks:**
- ❌ useRecordingHelpers: ✅ Has cleanup pattern (lines 465-498)
- ❌ JamTrack (via mediaSlice): ❌ No cleanup pattern

**Missing Pattern in mediaSlice.js:**

```javascript
// CURRENT (lines 234-244)
window.jamTrackDownloadProgress = (percent) => {
  dispatch(setDownloadState({ progress: percent }));
};
window.jamTrackDownloadSuccess = () => { /* ... */ };
window.jamTrackDownloadFail = (error) => { /* ... */ };

// SHOULD BE (following useRecordingHelpers pattern):
// 1. Store callback references in ref or state
// 2. Only delete if we still own the callback
// 3. Clean up on component unmount or new download
```

**Comparison to useRecordingHelpers pattern:**
```javascript
// useRecordingHelpers.js lines 465-498
const callbacksRef = useRef({});

useEffect(() => {
  if (window) {
    window.JK = window.JK || {};

    // Store our callback references
    callbacksRef.current = {
      HandleRecordingStartResult: handleRecordingStartResult,
      // ... more callbacks
    };

    // Register callbacks
    Object.assign(window.JK, callbacksRef.current);
  }

  return () => {
    // Only delete if we still own the callback
    if (window.JK && callbacksRef.current) {
      Object.keys(callbacksRef.current).forEach(key => {
        if (window.JK[key] === callbacksRef.current[key]) {
          delete window.JK[key];
        }
      });
    }
  };
}, [handleRecordingStartResult, /* ... */]);
```

**Risk:** MEDIUM (callback leak if multiple downloads or component remounts)

---

## 7. Performance Impact Assessment

### Critical Issues (Fix Now)

| Issue | Component | Impact | Priority |
|-------|-----------|--------|----------|
| No callback cleanup | mediaSlice.js (JamTrack download) | Memory leak, stale dispatch calls | HIGH |
| Sluggish controls | Metronome (BPM/sound/meter) | Poor UX, rapid native calls | HIGH |
| Missing ignore flag | Backing Track (duration fetch) | Potential state-after-unmount | MEDIUM |

### Non-Issues (No Action Needed)

| Pattern | Status | Notes |
|---------|--------|-------|
| Unbounded arrays | ✅ None found | Redux state arrays properly bounded |
| Timer cleanup | ✅ Present | Backing Track has proper cleanup |
| Interval cleanup | ✅ Present | Both Backing Track and JamTrack clean up intervals |

---

## 8. Recommended Fixes

### 8.1 Fix Metronome Sluggishness (HIGH PRIORITY)

**File:** `jam-ui/src/components/client/JKSessionMetronomePlayer.js`

**Changes:**
1. Add debounce pattern to `handleBpmChange`, `handleSoundChange`, `handleMeterChange`
2. Use ref to track pending updates
3. Add timer cleanup in useEffect

**Pattern:** Debounce with 300ms timeout (Option 1 from Section 5)

---

### 8.2 Fix JamTrack Callback Cleanup (HIGH PRIORITY)

**File:** `jam-ui/src/store/features/mediaSlice.js`

**Changes:**
1. Move window callback setup to component-level hook
2. Use callbacksRef pattern from useRecordingHelpers
3. Conditional cleanup (only delete if we own the callback)

**Alternative:** Create `useJamTrackDownload` hook that manages callbacks and thunk dispatch

---

### 8.3 Add Ignore Flag to Backing Track (MEDIUM PRIORITY)

**File:** `jam-ui/src/components/client/JKSessionBackingTrackPlayer.js`

**Changes:**
1. Add `let ignore = false;` at start of duration fetch useEffect (line 90)
2. Check `if (!ignore)` before setState calls (lines 129-131)
3. Return cleanup: `return () => { ignore = true; }`

**Pattern:** Matches JKSessionRecordingModal lines 54-81

---

### 8.4 Optional: Add Metronome Callback Cleanup (LOW PRIORITY)

**File:** `jam-ui/src/components/client/JKSessionMetronomePlayer.js`

**Rationale:** Metronome doesn't currently use window.JK callbacks, but if future features require native client callbacks (e.g., beat notifications), apply useRecordingHelpers pattern.

**Action:** Document pattern requirement for future development

---

## 9. Pattern Compliance Summary

### Established Patterns (v1.4/v1.5)

| Pattern | Reference | Components Following | Components Missing |
|---------|-----------|----------------------|--------------------|
| Timer cleanup | useRecordingHelpers:33-41 | Backing Track ✅ | Metronome ⚠️ (N/A), JamTrack ⚠️ (N/A) |
| Interval cleanup | useRecordingHelpers:33-41 | Backing Track ✅, JamTrack ✅ | Metronome ⚠️ (N/A) |
| Callback cleanup (window.JK) | useRecordingHelpers:465-498 | None | JamTrack ❌ (mediaSlice) |
| Ignore flags | JKSessionRecordingModal:54-81 | JamTrack ✅ (mountedRef) | Backing Track ❌, Metronome ⚠️ (N/A) |
| Visibility tracking | (Performance optimization) | All ✅ | None |

**Legend:**
- ✅ Pattern correctly applied
- ❌ Pattern missing (fix needed)
- ⚠️ Pattern not applicable (no timers/callbacks)

---

## 10. Test Coverage Recommendations

### Unit Tests (Jest)

**File:** `jam-ui/src/components/client/__tests__/JKSessionMetronomePlayer.test.js`

**Test cases:**
1. Verify BPM debounce (multiple rapid changes only trigger one jamClient call)
2. Verify timer cleanup on unmount
3. Verify callback stability (no recreation on unrelated state changes)

**File:** `jam-ui/src/store/features/__tests__/mediaSlice.test.js`

**Test cases:**
1. Verify download callbacks are cleaned up on unmount
2. Verify multiple download calls don't leak callbacks
3. Verify callback ownership check (don't delete if overwritten)

### Integration Tests (Playwright)

**File:** `jam-ui/test/media/metronome-controls.spec.ts`

**Test cases:**
1. Verify BPM slider is responsive (< 100ms lag)
2. Verify rapid slider changes don't hang UI
3. Verify metronome continues playing after control changes

**File:** `jam-ui/test/media/jamtrack-download-cancel.spec.ts`

**Test cases:**
1. Verify download can be cancelled mid-packaging
2. Verify navigation away during download doesn't leak callbacks
3. Verify second download doesn't interfere with first

---

## 11. Sources

**Code Files Analyzed:**
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/hooks/useRecordingHelpers.js` (pattern reference)
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionRecordingModal.js` (pattern reference)
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/store/features/mixersSlice.js` (state management reference)
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionMetronome.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionMetronomePlayer.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionBackingTrack.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionBackingTrackPlayer.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/components/client/JKSessionJamTrackPlayer.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/hooks/useMetronomeState.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/hooks/useMediaActions.js`
- `/Users/nuwan/Code/jam-cloud/jam-ui/src/store/features/mediaSlice.js`

**Pattern Establishment:**
- v1.4 phases (19-23): Timer cleanup, callback cleanup patterns
- v1.5 phases (24-25): Ignore flags, MAX_MESSAGES bounds

---

## 12. Confidence Assessment

| Area | Confidence | Rationale |
|------|------------|-----------|
| Cleanup patterns | HIGH | Complete code inspection with pattern comparison |
| Sluggishness cause | HIGH | Direct evidence from code (blocking await in onChange) |
| Risk assessment | HIGH | Based on established patterns and React lifecycle |
| Fix recommendations | HIGH | Patterns proven in v1.4/v1.5 |

**Overall Confidence:** HIGH

---

## Appendix A: Quick Reference Checklist

**Before submitting fix PR, verify:**

- [ ] All timers/intervals have cleanup in useEffect return
- [ ] All async useEffect operations have ignore flags
- [ ] All window.JK/window callbacks use callbacksRef pattern
- [ ] All event handlers with jamClient calls are debounced if high-frequency
- [ ] All state arrays have bounds or reset on lifecycle events
- [ ] Visibility tracking enabled for polling operations
- [ ] Tests cover cleanup behavior and performance regressions

**Pattern Files to Reference:**
- Timer cleanup: `useRecordingHelpers.js:33-41`
- Callback cleanup: `useRecordingHelpers.js:465-498`
- Ignore flags: `JKSessionRecordingModal.js:54-81`
- Debounce: (to be established in Metronome fix)

---

**End of Performance Audit**
