Skip to main content

ADR-003: GUI State Management Patterns

Status​

Accepted (with LLM Council amendments)

LLM Council Review​

Verdict: Conditional Acceptance (2026-01-30)

Amendments Required (integrated below):

  1. Pattern 2: Address async concurrency (race conditions)
  2. Pattern 4: Rename to "Atomic Replacement" to prevent UI flickering
  3. Add request correlation guidance for out-of-order responses
  4. Add JSON number precision handling at Tauri boundary

Context​

During LLM Council verification of v4.4.0, several technical debt issues were identified in conductor-gui/src-tauri/src/commands.rs related to state management:

  1. Unsafe defaults: validate_config defaults to valid: true when daemon response is malformed
  2. State desync: get_daemon_status doesn't update AppState when daemon returns application error
  3. Unsafe casts: Direct u64 as usize casts can silently truncate on 32-bit systems
  4. Stale flags: apply_connection_status doesn't clear previous connection flags on disconnect

These issues share common patterns: lack of fail-safe defaults, inconsistent state management, and missing input validation.

Decision​

Establish GUI state management patterns to prevent similar issues:

1. Fail-Safe Defaults Principle​

When parsing daemon responses, never default to success/valid states for missing data.

Bad:

let valid = data.get("valid").and_then(|v| v.as_bool()).unwrap_or(true);

Good:

let valid = data.get("valid")
    .and_then(|v| v.as_bool())
    .ok_or_else(|| "Daemon response missing 'valid' field")?;

Rationale: Silent success on malformed data can corrupt user configs or mask daemon issues.

2. AppState Synchronization Requirement​

Whenever communicating with the daemon, AppState must be updated to reflect IPC reachability.

Semantic Distinction (Council amendment v2):

  • AppState.daemon_connected: Tracks IPC reachability (can we reach the daemon process?)
  • DaemonStatus.connected: Tracks device connection status (is a MIDI device connected?)

These are distinct concepts. An error response from the daemon proves IPC succeeded.

Pattern: All IPC code paths must update AppState based on reachability:

  • IPC connection failure → state.set_daemon_connected(false)
  • IPC send failure → state.set_daemon_connected(false)
  • Response received (Success) → state.set_daemon_connected(true)
  • Response received (Error) → state.set_daemon_connected(true) (daemon is reachable!)
  • Connection refused/timeout → state.set_daemon_connected(false)

Async Concurrency Consideration (Council amendment): In Tauri's async environment, out-of-order responses can cause older data to overwrite newer data. For critical state updates:

  • Use Mutex or RwLock to ensure atomic read-modify-write
  • Consider request sequencing with monotonic IDs for long-running operations
  • Current implementation uses AppState with async locks which is sufficient for simple status updates

Rationale: UI components rely on AppState to know if the daemon is reachable. An error response proves connectivity; only IPC failures indicate unreachability.

3. Type-Safe Casting Pattern​

Use bounds-checked helper functions for all integer casts from external data:

/// Parse a port value from daemon response with bounds checking.
/// Returns None if value exceeds maximum valid MIDI port (255).
fn parse_port(value: u64) -> Option<usize> {
    const MAX_MIDI_PORT: u64 = 255;
    if value > MAX_MIDI_PORT {
        tracing::warn!("Port value {} exceeds maximum {}", value, MAX_MIDI_PORT);
        return None;
    }
    Some(value as usize)
}

JSON Number Precision (Council amendment): JavaScript uses IEEE-754 doubles, so integers > 2^53 lose precision before reaching Rust. For the current use case:

  • MIDI port values (0-255) are safely within this range
  • If future features require larger integers, transfer as strings and parse in Rust

Rationale: MIDI ports are 0-255. Values beyond this indicate daemon bug and should be rejected, not truncated.

4. Atomic Replacement Pattern for Collections​

When updating flags in collections, construct the new state atomically to prevent UI flickering:

fn apply_connection_status(mut devices: Vec<MidiDevice>, connected_port: Option<usize>) -> Vec<MidiDevice> {
    // Set all flags atomically in a single pass (prevents stale flags)
    for device in &mut devices {
        device.connected = connected_port.map_or(false, |port| device.index == port);
    }

    devices
}

Why "Atomic Replacement" not "Clear-Then-Set" (Council amendment): Clear-then-set causes UI flickering (list goes empty → list fills). The atomic pattern:

  • Computes new state in a single pass without intermediate empty state
  • Prevents "double-renders" in reactive UI frameworks
  • Maintains consistency without visual glitches

Rationale: Prevents stale state from previous calls while avoiding UI flicker.

Consequences​

Positive​

  • No silent failures masking invalid configurations
  • UI state always consistent with daemon responses
  • No silent data truncation on any platform
  • Clear disconnect behavior for users
  • Easier debugging when errors surface explicitly

Negative​

  • More verbose error handling code
  • May surface errors that were previously hidden (intentional)
  • Requires updating existing code to match patterns

Implementation​

Files to Update​

  • conductor-gui/src-tauri/src/commands.rs - Apply all 4 patterns
  • Add TDD tests for each fix

TDD Test Coverage​

// Fail-safe defaults
#[test] fn test_validate_config_missing_data_returns_error()

// AppState sync
#[test] fn test_daemon_status_error_updates_appstate()

// Type-safe casting
#[test] fn test_port_value_overflow_returns_none()

// Clear-then-set
#[test] fn test_apply_connection_clears_stale_flags()

References​

  • GitHub Epic: #25
  • Sub-issues: #26, #27, #28, #29
  • LLM Council v4.4.0 verification feedback