jellytau/SVELTE_CODE_REVIEW.md

Svelte Code Review: Logic That Should Be in Rust Backend

Executive Summary

The JellyTau architecture is generally well-designed with good separation of concerns. However, there are several areas where business logic and critical functionality are currently in the Svelte frontend that would be better placed in the Rust backend for reliability, testability, and maintainability.

Priority Summary:

• 🔴 High Priority (Move to Rust): Sync queue processing, offline sync logic
• 🟡 Medium Priority (Consider): Playback state transitions, device ID generation
• 🟢 Low Priority (Nice-to-have): Utility functions, validation logic

---

1. 🔴 HIGH PRIORITY: Sync Queue Processing Logic

Location: src/lib/services/syncService.ts

Issue

The entire sync queue processing system with retry logic, exponential backoff, and state management is implemented in Svelte, but this is core business logic that should be in Rust.

Current Implementation

// Lines 174-241: Entire queue processing with retry logic
- Polling for pending items
- Exponential backoff calculation
- Connectivity checks
- Retry tracking and failure marking
- Batch processing

Why This Should Be in Rust

1. Critical Business Logic - Retry logic and offline sync is essential for data integrity
2. Testing Difficulty - Hard to unit test async Tauri calls with timeouts and state
3. Reliability - Rust's error handling and type system better suit this logic
4. Consistency - Other backend systems use Rust exclusively
5. Performance - No need for Tauri async bridge for internal logic

Recommendation

Move SyncService to Rust backend:

• Create SyncProcessor struct in Rust that:
  • Manages sync queue processing
  • Implements exponential backoff
  • Handles retries with max retry limits
  • Manages batch processing
  • Integrates with connectivity monitor
• Svelte's syncService.ts becomes a thin wrapper that:
  • Provides queueMutation() to queue operations
  • Listens to sync progress events
  • Shows pending count in UI

Impact

• Effort: Medium (move ~150 lines of code + tests)
• Benefits: Better reliability, testability, consistency
• Risk: Low (well-isolated logic)

---

2. 🔴 HIGH PRIORITY: Playback Reporting Connectivity Logic

Location: src/lib/services/playbackReporting.ts

Issue

Playback reporting contains duplicated connectivity checks and offline queuing logic repeated across multiple functions:

// Lines 45-52: In reportPlaybackStart()
if (!get(isServerReachable)) {
  await syncService.queueMutation(...);
  return;
}

// Lines 109-113: Same pattern in reportPlaybackProgress()
if (!get(isServerReachable)) {
  return;
}

// Lines 151-158: Same pattern in reportPlaybackStopped()
if (!get(isServerReachable)) {
  await syncService.queueMutation(...);
  return;
}

Why This Should Be in Rust

1. Duplicated Logic - Same connectivity check pattern repeated 3+ times
2. Decision Making - Should backend decide whether to queue vs report?
3. Consistency - Centralize offline handling strategy
4. Type Safety - Rust enums for operation types better than strings

Recommendation

Move all playback reporting to Rust backend:

• Create PlaybackReporter in Rust that:
  • Handles report_playback_start(), progress(), stopped()
  • Internally decides online vs offline path
  • Manages queuing for sync
  • Throttles frequent updates (already done in throttle.rs)
• Svelte becomes simple: Call invoke("player_report_playback_start", {...})
• No need for isServerReachable checks in Svelte

Current vs Proposed

Current (Svelte):

const repo = auth.getRepository();
if (isOnline) {
  await repo.reportPlaybackStart(...);
  await invoke("storage_mark_synced", ...);
} else {
  await syncService.queueMutation(...);
}

Proposed (Rust Command):

#[tauri::command]
async fn player_report_playback_start(
  item_id: String,
  position: u64,
) -> Result<(), String> {
  // Rust backend decides everything
  // Returns success/queued status
}

Impact

• Effort: Medium (refactor ~100 lines across services)
• Benefits: Removes duplicated logic, centralizes offline strategy
• Risk: Low (clear command boundaries)

---

3. 🟡 MEDIUM PRIORITY: Playback State Transition Logic

Location: src/lib/services/playerEvents.ts

Issue

Complex state transition logic is in Svelte event handlers:

// Lines 172-224: handleStateChanged()
// - Mode switching (local vs remote)
// - Queue status updates
// - Context-dependent state logic
// - Preload triggering

// Lines 100-105: Filter logic for remote vs local events
const mode = get(playbackMode);
if (mode.mode === "remote" && !mode.isTransferring) {
  return;
}

Current Problems

1. Mode Switching Logic (lines 180-185, 213-218)

   // Should backend manage this?
   if (mode.mode !== "local") {
     playbackMode.setMode("local");
   }
   

2. Queue Status Updates (lines 230-247)

   • Called on state changes to sync hasNext, hasPrevious, shuffle, repeat
   • Could be included in player events directly

3. Event Filtering (lines 100-105)

   • Decides to skip local events during remote playback
   • Should backend send these events at all?

Why This Might Belong in Rust

1. State Machine - Playback has clear states that could be managed centrally
2. Consistency - Remote vs local mode logic is scattered
3. Testing - State transitions are hard to unit test across Tauri boundary

Recommendation (Consider)

This is a refactoring consideration, not urgent:

• ✅ Keep in Svelte: Event listening and store updates (current location is fine)
• ✅ Keep in Svelte: Mode display logic in components
• 🤔 Consider Moving: Mode state machine logic to Rust (but current approach works)
• 🤔 Consider: Including queue status in player events instead of separate invoke

Alternative: Optimize Current Approach

If keeping in Svelte, improve playerEvents.ts:

1. Extract mode logic into separate module
2. Include queue status in PlayerStatusEvent from Rust
3. Add unit tests for state transitions

Impact

• Effort: Medium-High (significant refactoring)
• Benefits: Clearer state machine, easier testing
• Risk: Medium (affects playback flow)
• Priority: Lower than sync issues above

---

4. 🟡 MEDIUM PRIORITY: Device ID Generation

Location: src/lib/services/deviceId.ts

Issue

UUID v4 generation is in Svelte, but persistence is in Rust:

// Lines 15-21: UUID generation in TypeScript
function generateUUID(): string {
  return "xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx".replace(/[xy]/g, function (c) {
    const r = (Math.random() * 16) | 0;
    const v = c === "x" ? r : (r & 0x3) | 0x8;
    return v.toString(16);
  });
}

// Lines 35-54: Then calls Rust to persist
const deviceId = await invoke<string | null>("device_get_id");
if (!deviceId) {
  const newDeviceId = generateUUID();  // <- Generated in Svelte
  await invoke("device_set_id", { deviceId: newDeviceId });
}

Problems

1. Split Responsibility - Generation in Svelte, persistence in Rust
2. Multiple Generation Points - Could generate different IDs on different app starts if Rust storage fails
3. Simple Logic - UUID generation should be one place

Recommendation

Move device ID to Rust:

• Change device_get_id to return existing ID or generate+store new one atomically
• Svelte just calls await invoke("device_get_id") once

// In Rust
#[tauri::command]
async fn device_get_id(storage: State<'_, Storage>) -> Result<String> {
  if let Some(id) = storage.get_device_id()? {
    return Ok(id);
  }

  // Generate and store atomically
  let id = uuid::Uuid::new_v4().to_string();
  storage.set_device_id(&id)?;
  Ok(id)
}

Impact

• Effort: Low (simple change)
• Benefits: Single responsibility, atomic operation
• Risk: Very low

---

5. 🟢 LOW PRIORITY: Server Reachability Reload Logic

Location: src/lib/composables/useServerReachabilityReload.ts

Issue

Tracks when server becomes reachable to reload data:

// Lines 44-52: checkServerReachability()
if (isServerReachable && !previousServerReachable && hasLoadedOnce) {
  reloadFn();
}

Current Status

✅ This is actually fine to stay in Svelte because:

• It's UI-specific (reload on screen becomes visible)
• Simple stateless logic
• Works well as a composable

Note

The backend's connectivity:reconnected event (listened to in src/lib/stores/connectivity.ts:64) is the right abstraction level.

---

6. 🟢 LOW PRIORITY: Input Validation & Type Conversions

Location:

• src/lib/utils/validation.ts
• src/lib/utils/jellyfinFieldMapping.ts
• src/lib/api/conversions.ts

Current Status

✅ These are fine to stay in Svelte because:

• UI validation (email format, username length)
• Display formatting (duration, field mapping)
• Jellyfin API type conversions for UI display
• Svelte-only concerns (component state)

Keep As-Is

No action needed. These are thin utility layers for UI concerns.

---

Summary Table

Component	Current	Should Move?	Priority	Effort	Risk
syncService.ts	Svelte	→ Rust	🔴 High	Medium	Low
playbackReporting.ts	Svelte	→ Rust	🔴 High	Medium	Low
playerEvents.ts (state logic)	Svelte	Consider	🟡 Medium	Medium	Medium
deviceId.ts	Svelte	→ Rust	🟡 Medium	Low	Very Low
useServerReachabilityReload.ts	Svelte	Keep ✅	-	-	-
validation.ts	Svelte	Keep ✅	-	-	-
API conversions	Svelte	Keep ✅	-	-	-

---

Recommended Implementation Order

1. Phase 1 (High Impact, Low Risk)

   • Move device ID generation to Rust (1-2 hours)
   • This is small but improves robustness

2. Phase 2 (High Impact, Medium Effort)

   • Move sync queue processing to Rust (4-6 hours)
   • Move playback reporting logic to Rust (4-6 hours)
   • These are related and can be done together

3. Phase 3 (Optional, More Complex)

   • Consider state machine refactoring in playerEvents.ts
   • Only if experiencing issues or during major refactor

---

Architecture Principles to Maintain

When implementing these changes, preserve:

1. Command-Based API - Svelte invokes Rust commands, doesn't call internal functions
2. Event-Driven Updates - Rust emits events for state changes, Svelte listens
3. Thin Frontend - Svelte only handles UI rendering and user input
4. Type Safety - Use Rust enums and structs for critical logic
5. Offline-First - Backend decides online vs offline paths, not frontend

---

Questions & Notes

• Q: Should repository.ts API calls move to Rust?

  • A: No - RepositoryClient is a good abstraction layer. Keep as-is.

• Q: Should UI state like showSleepTimerModal be in Rust?

  • A: No - UI state belongs in Svelte stores. This is correct.

• Q: Should we move all HTTP calls to Rust?

  • A: The Jellyfin HTTP client is already in Rust. RepositoryClient wraps it, which is fine.

• Q: What about preload logic?

  • A: preload.ts is fine - it's a simple command wrapper + orchestration.

---

Files with Findings

• playerEvents.ts - State logic, event filtering
• playbackReporting.ts - Offline logic duplication
• syncService.ts - Sync queue processing
• deviceId.ts - Split responsibility
• useServerReachabilityReload.ts - Fine as-is
• appState.ts - Fine as-is (UI state)