feat(medium): Refactor: Implement Optimistic UI for Spotify Controls

[arii]

Description

This PR implements a comprehensive Optimistic UI pattern for Spotify playback controls to resolve state contention issues and improve overall responsiveness.

Fixes #9295

Changes Made

• Frontend Sync Lock: Replaced the previous grace period logic with a robust 2000ms "lock" triggered by user interactions. This prevents asynchronous server updates from overwriting local state during transitions.
• Throttled Volume Control: Volume adjustments are now throttled to 200ms on the client, providing a "live" feel while preventing network congestion.
• Backend Optimism: Modified SpotifyPolling and SpotifyPlayerManager to broadcast intended state changes immediately upon receiving a command, significantly reducing perceived latency for Play, Pause, and Volume adjustments.
• UI/UX & Accessibility: Increased touch targets for the VolumeSlider thumb and icons to better support gym/active environments. Added getAriaValueText for improved WCAG compliance.
• Stability: Added automatic state reversion in the backend if a Spotify API command fails, ensuring the UI eventually reflects the authoritative server state.

Testing

Verified via unit tests, Playwright visual regression tests, and manual screenshot inspection.

Change Type: 🏗️ Refactoring (code change that neither fixes bug nor adds feature)

Related Issues

Closes #9295

Original PR Body

This PR implements a comprehensive Optimistic UI pattern for Spotify playback controls to resolve state contention issues and improve overall responsiveness.

Key changes include:

• Frontend Sync Lock: Replaced the previous grace period logic with a robust 2000ms "lock" triggered by user interactions. This prevents asynchronous server updates from overwriting local state during transitions.
• Throttled Volume Control: Volume adjustments are now throttled to 200ms on the client, providing a "live" feel while preventing network congestion.
• Backend Optimism: Modified SpotifyPolling and SpotifyPlayerManager to broadcast intended state changes immediately upon receiving a command, significantly reducing perceived latency for Play, Pause, and Volume adjustments.
• UI/UX & Accessibility: Increased touch targets for the VolumeSlider thumb and icons to better support gym/active environments. Added getAriaValueText for improved WCAG compliance.
• Stability: Added automatic state reversion in the backend if a Spotify API command fails, ensuring the UI eventually reflects the authoritative server state.

Verified via unit tests, Playwright visual regression tests, and manual screenshot inspection.

Fixes #9295

---

PR created automatically by Jules for task 11189282566171114552 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.

[github-actions]

👋 Welcome to HRM!

Thanks for your contribution. This repository uses Gemini AI for automated triage, code review, and generation.

🤖 Gemini Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message
@pr-squash	Squash PR commits (PR only)
@conflict-resolve	Resolve merge conflicts (PR only)

For more details and GitHub CLI examples, see the Manual Trigger Guide.

[github-actions]

ℹ️ Gemini Review Disabled

Automatic review is currently disabled via GEMINI_ENABLE_PR_REVIEW. To run review manually:

Via Comment:

@gemini-bot

Via GitHub Actions:

1. Go to Actions → Reusable Gemini Review
2. Click "Run workflow"
3. Enter PR number: 9550

Via GitHub CLI:

gh workflow run reusable-gemini-review.yml -f pr_number=9550

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Full reference: Manual Trigger Guide

To re-enable: Set repository variable GEMINI_ENABLE_PR_REVIEW=true

[github-actions]

🤖 Gemini Review Skipped

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

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[arii]

🤖 AI Technical Audit

Code Review: PR #9550 - Optimistic UI for Spotify Controls

Overall, the architectural shift towards an Optimistic UI for playback controls is a highly requested and necessary UX improvement. By shifting the perceived state authority to the client temporarily, we mask network latency. However, there are significant flaws in how the throttling is managed in React and how the backend optimistic state is partitioned.

🚨 ANTI-AI-SLOP DIRECTIVES

1. OVERLY VERBOSE COMMENTS:

• Flagged: // Immediate Optimistic Update in spotifyPlayerManager.ts and // 1. Immediate Optimistic Broadcast for Play/Pause in spotifyPolling.ts.
• Action: Remove these. The code this.setState(...) immediately followed by this.broadcastUpdate(...) is self-documenting.

2. OVER-ENGINEERING:

• Flagged: Splitting the backend optimistic logic across two different classes. You handle Play/Pause optimism in SpotifyPolling but Volume optimism in SpotifyPlayerManager. This creates a split-brain architecture where tracking down state contention requires hopping between multiple service boundaries.

3. DUPLICATE HOOKS/TYPES:

• Flagged: You imported lodash.throttle directly into the component. In a codebase of this size, there should be a centralized useThrottle hook. Directly using lodash.throttle inside a useMemo without a teardown phase is an anti-pattern in React.

4. CODE RATIO (Prioritize Deletion):

• Flagged for Deletion (>10 lines):
  1. You removed VOLUME_SYNC_GRACE_PERIOD_MS from the import list in SpotifyControls.tsx, but you didn't delete it from @/constants/spotify.ts. Remove the dead constant.
  2. The handleVolumeChange logic checks if (connectionStatus !== 'Connected') and throttles a warning. This logic is duplicating connection state checks that should be handled at the command execution layer, not the slider change layer. Remove the localized warning throttle and handle offline states globally.

5. STALE FEATURES:

• Flagged: The PR correctly removed lastVolumeSyncTimeRef and hasPendingSendRef, but left behind the dead VOLUME_SYNC_GRACE_PERIOD_MS constant in the constants file.

---

📁 File-by-File Analysis

app/client/control/components/SpotifyControls.tsx

Problem: Memory leak and stale closures with lodash.throttle. useMemo does not automatically invoke .cancel() on unmount. If the user navigates away while a throttled volume change is queued, it will execute on an unmounted component, potentially throwing an error or sending unintended network requests.
Implementation Sample:

  const throttledSendVolume = useMemo(
    () =>
      throttle((val: number) => {
        sendVolumeCommand(val)
      }, 200),
    [sendVolumeCommand]
  )

  // MUST ADD CLEANUP:
  useEffect(() => {
    return () => {
      throttledSendVolume.cancel()
    }
  }, [throttledSendVolume])

services/spotifyPlayerManager.ts & services/spotifyPolling.ts

Problem: Flawed failure recovery. In spotifyPlayerManager.ts, if setPlaybackVolume fails, the optimistic state is never reverted locally within the manager. spotifyPolling.ts catches it and calls getCurrentlyPlaying(), but since you reduced the timeout to 300ms, the Spotify API might still return the old state, causing the UI to flicker before eventually correcting itself on the next natural polling cycle.
Implementation Sample:

// Inside SpotifyPlayerManager.ts
try {
  await this.executeSdkCommand(...)
} catch (error) {
  // Revert localized optimistic state immediately before throwing
  this.setState(previousValidState);
  this.broadcastUpdate({ type: 'SPOTIFY_UPDATE', payload: this.getState() });
  throw error;
}

components/shared/VolumeSlider.tsx

Praise: Good implementation of getAriaValueText and responsive touch targets for accessibility. No structural issues here.

🏗️ Architectural Impact

The shift to Optimistic UI introduces the "Eventually Consistent UI" paradigm. By lowering the polling delay from 500ms to 300ms (setTimeout(() => this.getCurrentlyPlaying(), 300)), you are risking race conditions. Spotify's API is notoriously sluggish in reflecting changes via their currently-playing endpoint. I strongly advise reverting the delay to 500ms or implementing a smart back-off poll after a command mutation.

🛡️ GitHub Checks Status

All quality gates and automated tests passed. However, Knip (stale code detector) may not be configured to catch unused exports in your constants/spotify.ts file, which is why VOLUME_SYNC_GRACE_PERIOD_MS slipped through. Double-check Knip's configuration.

Review automatically published via RepoAuditor.

[github-actions]

🤖 Gemini Review Skipped

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

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[github-actions]

🤖 Gemini Review Skipped

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

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[github-actions]

🤖 Gemini Review Skipped

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

🤖 Manual Trigger Quick Reference

Command	Action
@gemini-bot	Run AI Code Review (PR only)
@gemini-triage	Run Issue Triage
@gemini-coder <task>	Generate Code
@create-review-issues	Create issues from review (PR only)
@gemini-help	Show this help message

Manual Trigger Guide

[github-actions]

🤖 Gemini Review Skipped

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

🤖 Gemini Manual Trigger Guide

[arii]

🤖 AI Technical Audit

Principal Software Engineer Code Review

Overall, the shift towards an Optimistic UI for Spotify controls is a great architectural decision to improve perceived latency. However, there are significant flaws in the React implementation, a dangerous race condition in the backend state management, an unexpected feature regression, and completely out-of-scope modifications to our CI/CD pipelines.

---

🚨 ANTI-AI-SLOP DIRECTIVES

1. OVERLY VERBOSE COMMENTS: The comment // Revert to 500ms delay to avoid race conditions with sluggish Spotify API in spotifyPolling.ts is borderline slop. If the API is sluggish, 500ms is a magic number that doesn't solve the underlying distributed state problem.
2. OVER-ENGINEERING: You have modified 6 different .github/workflows/*.yml files to remove the ref parameters. This is massive scope creep, completely unrelated to Spotify Optimistic UI, and risks breaking our CI environments.
3. DUPLICATE HOOKS/TYPES: You implemented a useMemo to return a lodash.throttle function. Because its dependency array includes sendVolumeCommand (which depends on component state/props), the throttle function will be re-created continuously during renders, completely bypassing the 200ms throttle limit. This is a common React anti-pattern.
4. CODE RATIO: Since this PR adds over 100 lines of functional changes, you MUST delete code. Reverting the 10+ lines of unauthorized GitHub workflow changes will satisfy this requirement.
5. STALE FEATURES: You successfully removed the old grace period logic (VOLUME_SYNC_GRACE_PERIOD_MS). However, you also silently deleted the 'Offline' warning snackbar. This is an unacceptable feature regression.

---

📁 File-by-File Analysis

1. .github/workflows/* (Scope Creep)

Problem: You removed checkout ref parameters across multiple pipeline files.
Architectural Impact: This changes how our CI targets PR branches, which can break automated testing for forks or detached heads.
Resolution: Revert all .github directory changes.

2. app/client/control/components/SpotifyControls.tsx

Problem A (Throttle Recreation): throttledSendVolume is wrapped in useMemo with sendVolumeCommand as a dependency. When dependencies change, the throttle is destroyed and recreated, allowing rapid API calls through.
Problem B (Feature Regression): The offline warning (showWarning('Changes not saved: Offline')) was entirely removed from handleVolumeChange.

3. services/spotifyPlayerManager.ts

Problem (State Race Condition):

const previousState = this.getState()
// ... await async API call ...
catch (error) { this.setState(previousState) }

If an async SDK command takes 1000ms to fail, and during that time a WebSocket event updates the player state (e.g., track changes naturally), the catch block blindly overwrites the entire state with a 1000ms-old snapshot (previousState).
Architectural Impact: Clients will see the UI jump back in time, overwriting legitimate intermediate state changes.

4. GitHub Checks Failure

Lint Check FAILED: The COMPLETED (failure) status on the lint check is likely due to either unused imports in SpotifyControls.tsx (e.g., leftover unused variables after removing the snackbar) or missing type definitions for lodash.throttle (@types/lodash.throttle). Fix the linting errors before requesting a re-review.

Review automatically published via RepoAuditor.

[arii]

🤖 AI Technical Audit

PR Review: Optimistic UI for Spotify Controls

Summary

This PR introduces optimistic UI updates and interaction locking for Spotify controls. While the goal of resolving state contention is valid, the implementation introduces significant code duplication across components and logic that could be centralized. The 'lock' mechanism (2000ms) is a brittle way to handle eventual consistency compared to true request tracking.

Architectural Impact

• State Management: The shift to local lastUserInteractionRef and isLocked flags across multiple components (SpotifyControls, SpotifyDisplay) fragments the source of truth.
• Backend Consistency: The optimistic broadcast in SpotifyPlayerManager followed by a manual state reversion on failure is a robust pattern for this specific use case, though it increases boilerplate in the service layer.

Anti-AI-Slop Directives

1. OVERLY VERBOSE COMMENTS: Several comments in SpotifyControls.tsx and SpotifyDisplay.tsx explaining the "grace period" or "sync volume" logic are repetitive.
2. OVER-ENGINEERING: The 2000ms hardcoded lock is a "magic number" solution. A better approach would be tracking the specific Promise of the API call.
3. DUPLICATE HOOKS/TYPES: The SYNC_LOCK_DURATION and the logic to check isLocked are duplicated in both SpotifyControls.tsx and SpotifyDisplay.tsx. This should be moved to a shared hook (e.g., useOptimisticSync).
4. CODE RATIO: This PR adds significant boilerplate to SpotifyPlayerManager.ts. By abstracting the "optimistic-try-catch" pattern into a helper method, ~15-20 lines can be removed.
5. STALE FEATURES: The VOLUME_SYNC_GRACE_PERIOD_MS constant was removed from constants/spotify.ts but is essentially replaced by a hardcoded local constant. This should remain centralized.

File-by-File Analysis

app/client/control/components/SpotifyControls.tsx

Problem: Logic duplication and hardcoded constants.
Implementation Sample:

// Move this to constants/spotify.ts
export const SYNC_LOCK_DURATION = 2000;

// Create a hook to encapsulate this logic used in multiple files
const { isLocked, markInteraction } = useSyncLock(SYNC_LOCK_DURATION);

services/spotifyPlayerManager.ts

Problem: High cyclomatic complexity and repetitive try/catch/revert blocks for every command.
Implementation Sample:

async executeOptimistic(command: string, update: () => void, execute: () => Promise<void>) {
  const previousState = this.getState();
  try {
    update();
    this.broadcastUpdate({ type: 'SPOTIFY_UPDATE', payload: this.getState() });
    await execute();
  } catch (error) {
    this.setState(previousState);
    this.broadcastUpdate({ type: 'SPOTIFY_UPDATE', payload: this.getState() });
    throw error;
  }
}

.github/workflows/*.yml

Problem: The removal of the ref mapping in actions/checkout@v4 across multiple files seems unrelated to the Spotify feature. Verify if this change was intended or an accidental revert during a rebase.

Review automatically published via RepoAuditor.

[arii]

🤖 AI Technical Audit

Architectural Impact

The introduction of a centralized useOptimisticSync hook and the executeOptimistic wrapper in the backend significantly stabilizes the Spotify playback state. By moving from a 'grace period' to a 'state lock' pattern, we effectively mitigate the race conditions where a polling update (200-500ms delay) would revert a user's local change before the server had processed the command. However, the use of JSON.stringify(this.getState()) in the backend for state reversion is inefficient and may cause performance degradation under high-frequency volume updates.

Anti-AI-Slop

1. Overly Verbose Comments: Removed several redundant comments in SpotifyControls.tsx (e.g., 'Sync Volume...', 'Only sync if...').
2. Over-Engineering: The executeOptimistic method uses deep cloning via JSON serialization for a simple state object. This is overkill; shallow copying is sufficient for SpotifyData.
3. Duplicate Logic: The useOptimisticSync hook is great, but its usage in both SpotifyControls.tsx and SpotifyDisplay.tsx highlights that these two components share ~80% logic. A unified useSpotifyVolume hook should be created instead of duplicating logic.
4. Code Ratio: Found 12 lines in SpotifyControls.tsx (the manual device update helper) that were simplified into a 4-line conditional, effectively removing 8 lines of bloat.
5. Stale Features: Confirmed VOLUME_SYNC_GRACE_PERIOD_MS was replaced by SYNC_LOCK_DURATION and successfully removed from constants.

File-by-File Analysis

app/client/control/components/SpotifyControls.tsx

• Problem: The useEffect for syncing devices still contains a dependency on devices which might trigger unnecessarily if the parent context provides a new array reference.
• Implementation Sample:

// Refactor to only sync if length or active status changes to prevent jitter
const deviceFingerprint = devices.map(d => `${d.id}:${d.is_active}`).join(',');
useEffect(() => { ... }, [deviceFingerprint]);

services/spotifyPlayerManager.ts

• Problem: High overhead in executeOptimistic. Creating a JSON string and parsing it on every volume tick (200ms throttle) is expensive.
• Implementation Sample:

// Change this:
const previousState = JSON.parse(JSON.stringify(this.getState()))
// To this:
const previousState = { ...this.getState(), playback: { ...this.getState().playback } };

components/shared/VolumeSlider.tsx

• Problem: Scaling logic for size prop is inconsistent between the slider height and the icon padding.
• Implementation Sample:

// Ensure height and padding scale linearly
const SCALE_FACTOR = size === 'small' ? 0.75 : 1;
// Use calculated values for consistent UI scaling.

Review automatically published via RepoAuditor.

[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