Fix Bluetooth & Spotify Improvements (PR #9662)

[arii]

Repairs PR #9662 by incorporating the fixes requested by the principal engineer.

• app/client/control/components/SpotifyControls.tsx: The explicit clear of optimisticIsPlaying inside the useEffect was removed to rely solely on the 2.5s timer (playbackGraceTimerRef), reducing flicker and preventing race conditions, and unneeded comments were deleted.
• hooks/usePersistentStorage.ts: Swapped out the expensive JSON.stringify checks for isEqual from lodash, improving the performance on every render.
• context/UserSettingsContext.tsx: Simplified the migratePreferences function to map over a list of numericFields directly, removing the redundant single-field parses. Also replaced JSON.stringify with isEqual.
• hooks/useBluetoothHRM.ts: Ensured no hidden string default timeouts or fallback literals by substituting the hardcoded 20000 timeout parameter inside cancellablePromise with the canonical DATA_LIVENESS_TIMEOUT_MS. Restored lastWatchdogMark.current = 0 to preserve expected watchdog tracking behavior in unit tests.

---

PR created automatically by Jules for task 12597848218781664933 started by @arii

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[arii]

🤖 AI Technical Audit

PR Review: Bluetooth & Spotify Improvements

Architectural Impact

This PR introduces optimistic UI updates to the Spotify integration and shifts the storage strategy from synchronous initialization to an effect-based hydration model. While the optimistic UI improves perceived performance, the changes in usePersistentStorage may introduce a "flash of initial state" where initialValue is rendered before the stored value is loaded in useEffect. The move to lodash/isEqual across the board is a positive architectural shift for performance, reducing the overhead of JSON.stringify on every render cycle.

File-by-File Analysis

app/client/control/components/SpotifyControls.tsx

• Problem: The optimistic state optimisticIsPlaying is introduced but there's no mechanism to handle failed API requests. If sendSpotifyCommand fails, the UI will show the wrong state for 2.5 seconds.
• Implementation Sample:

// Recommendation: Reset optimistic state on error
try {
  await sendSpotifyCommand(command);
} catch (e) {
  setOptimisticIsPlaying(null);
  if (playbackGraceTimerRef.current) clearTimeout(playbackGraceTimerRef.current);
}

hooks/usePersistentStorage.ts

• Problem: The PR changes the hook to always initialize with initialValue and load from storage in useEffect. This breaks SSR/Hydration consistency and causes an extra render for every component using this hook.
• Implementation Sample: Keep the synchronous check for typeof window !== 'undefined' in the initializer to avoid the unnecessary mount-then-update cycle.

context/UserSettingsContext.tsx

• Problem: The migration logic for numericFields is repetitive and uses parseFloat without checking for empty strings or non-numeric types effectively.
• Implementation Sample:

const numericFields: Array<keyof UserPreferences> = ['userAge', 'userWeight', 'userHeight'];
if (numericFields.includes(k as keyof UserPreferences)) {
  const num = Number(storedVal);
  (acc as any)[k] = !isNaN(num) ? num : initialPreferences[k as keyof UserPreferences];
}

hooks/useBluetoothHRM.ts

• Problem: Adding the isConnecting.current check in the watchdog is a good safety measure, but dataLivenessTimeoutMs is now defaulting to 30s (DATA_LIVENESS_TIMEOUT_MS) which is significantly higher than the previous 10s. This might delay error detection for users.

ANTI-AI-SLOP DIRECTIVES

1. OVERLY VERBOSE COMMENTS: // Cleanup timers on unmount in SpotifyControls.tsx is redundant. Standard useEffect cleanup patterns are self-explanatory to engineers.
2. OVER-ENGINEERING: The migratePreferences function is becoming a manual mapper. Using a schema validator like Zod or Joi would be cleaner than hardcoding numericFields arrays.
3. DUPLICATE TYPES: The numericFields array in UserSettingsContext.tsx manually lists keys already defined in the UserPreferences interface. This should be derived via keyof or a constant object to avoid drift.
4. CODE RATIO: Found 12 lines that can be removed:
   • app/client/control/components/SpotifyControls.tsx: Lines 228-233 (The manual unmount cleanup is often unnecessary if the timer is managed within the command logic itself).
   • hooks/usePersistentStorage.ts: Lines 134-142 (The redundant loadFromStorage call inside the keyChanged block can be simplified).
5. STALE FEATURES: Verify that VOLUME_SYNC_GRACE_PERIOD_MS (updated to 3000ms) doesn't conflict with the new 2500ms playbackGraceTimerRef. One should likely drive the other.

Review automatically published via RepoAuditor.

[google-labs-jules]

🤖 AI Technical Audit

PR Review: Bluetooth & Spotify Improvements

Architectural Impact

This PR introduces optimistic UI updates to the Spotify integration and shifts the storage strategy from synchronous initialization to an effect-based hydration model. While the optimistic UI improves perceived performance, the changes in usePersistentStorage may introduce a "flash of initial state" where initialValue is rendered before the stored value is loaded in useEffect. The move to lodash/isEqual across the board is a positive architectural shift for performance, reducing the overhead of JSON.stringify on every render cycle.

File-by-File Analysis

app/client/control/components/SpotifyControls.tsx

• Problem: The optimistic state optimisticIsPlaying is introduced but there's no mechanism to handle failed API requests. If sendSpotifyCommand fails, the UI will show the wrong state for 2.5 seconds.
• Implementation Sample:

// Recommendation: Reset optimistic state on error
try {
  await sendSpotifyCommand(command);
} catch (e) {
  setOptimisticIsPlaying(null);
  if (playbackGraceTimerRef.current) clearTimeout(playbackGraceTimerRef.current);
}

hooks/usePersistentStorage.ts

• Problem: The PR changes the hook to always initialize with initialValue and load from storage in useEffect. This breaks SSR/Hydration consistency and causes an extra render for every component using this hook.
• Implementation Sample: Keep the synchronous check for typeof window !== 'undefined' in the initializer to avoid the unnecessary mount-then-update cycle.

context/UserSettingsContext.tsx

• Problem: The migration logic for numericFields is repetitive and uses parseFloat without checking for empty strings or non-numeric types effectively.
• Implementation Sample:

const numericFields: Array<keyof UserPreferences> = ['userAge', 'userWeight', 'userHeight'];
if (numericFields.includes(k as keyof UserPreferences)) {
  const num = Number(storedVal);
  (acc as any)[k] = !isNaN(num) ? num : initialPreferences[k as keyof UserPreferences];
}

hooks/useBluetoothHRM.ts

• Problem: Adding the isConnecting.current check in the watchdog is a good safety measure, but dataLivenessTimeoutMs is now defaulting to 30s (DATA_LIVENESS_TIMEOUT_MS) which is significantly higher than the previous 10s. This might delay error detection for users.

ANTI-AI-SLOP DIRECTIVES

1. OVERLY VERBOSE COMMENTS: // Cleanup timers on unmount in SpotifyControls.tsx is redundant. Standard useEffect cleanup patterns are self-explanatory to engineers.
2. OVER-ENGINEERING: The migratePreferences function is becoming a manual mapper. Using a schema validator like Zod or Joi would be cleaner than hardcoding numericFields arrays.
3. DUPLICATE TYPES: The numericFields array in UserSettingsContext.tsx manually lists keys already defined in the UserPreferences interface. This should be derived via keyof or a constant object to avoid drift.
4. CODE RATIO: Found 12 lines that can be removed:
   • app/client/control/components/SpotifyControls.tsx: Lines 228-233 (The manual unmount cleanup is often unnecessary if the timer is managed within the command logic itself).
   • hooks/usePersistentStorage.ts: Lines 134-142 (The redundant loadFromStorage call inside the keyChanged block can be simplified).
5. STALE FEATURES: Verify that VOLUME_SYNC_GRACE_PERIOD_MS (updated to 3000ms) doesn't conflict with the new 2500ms playbackGraceTimerRef. One should likely drive the other.

Review automatically published via RepoAuditor.

Acknowledged.

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide

[github-actions]

🤖 Gemini Review Skipped

Review was skipped for the following reason: Gemini review is globally disabled

🤖 Gemini Manual Trigger Guide