Integrate MLX for on-device AI assistant and multi-platform streaming

[arkavo-com]

No description provided.

[superninja-app]

Code Review: PR #239 — Integrate MLX for on-device AI assistant and multi-platform streaming

Summary

This is a large PR (+4900/-998 across 44 files) that introduces three major feature areas:

1. MLX On-Device AI Integration — Adds Apple MLX framework support for local LLM inference via MuseCore, enabling three AI "Muse" roles: Producer (stream monitoring), Publicist (content creation), and Sidekick (on-camera avatar).
2. Multi-Platform Simulcast Streaming — Refactors the RTMP streaming stack from single-destination to multi-destination (simulcast) with per-platform stream key management.
3. YouTube Live API Integration — Full YouTube broadcast lifecycle: create/bind/transition broadcasts, live chat polling, Super Chat events, and OAuth scope upgrades.

Additionally includes: UI refresh with glassmorphism design, Swift 6.3 version bump, Twitch EventSub WebSocket client, token refresh flow, feedback UX refactor, and feature flag updates.

---

Strengths

Architecture & Design

• Clean role separation: The Producer/Publicist/Sidekick role pattern is well-designed with clear boundaries. AvatarRole, RolePromptProvider, and PlatformContext provide excellent extensibility.
• Protocol-oriented platform contexts: PlatformContext protocol with per-platform structs (BlueskyContext, YouTubeContext, etc.) is a clean, maintainable pattern.
• MLX integration is well-layered: MLXBackend → MLXResponseProvider → ModelManager → PublicistViewModel is a clean dependency chain.
• Backward compatibility preserved: StreamViewModel maintains computed property shims (selectedPlatform, streamKey) for single-platform code paths while adding multi-platform support.
• Feature-flagged rollout: FeatureFlags.localAssistant gates the new MLX features, allowing safe incremental rollout.

Code Quality

• Consistent use of modern Swift patterns: @Observable, AsyncStream, AsyncThrowingStream, structured concurrency, and Mutex for thread safety.
• Good prompt engineering: System prompts for each role are well-crafted with clear boundaries, safety guidelines, and bilingual support (EN/JA).
• Accessibility identifiers: UI elements like "Platform_Bluesky", "Btn_Generate", "Source_Face" are properly tagged for UI testing.
• Good error handling in streaming: The RTMP simulcast fan-out properly handles per-destination errors without killing other streams.

Testing

• UI tests for both Producer and Publicist features cover core navigation and element existence.
• Tests use proper waitForExistence patterns for async UI.

---

Issues / Concerns

🔴 Critical

1. Debug print statements left in production audio encoder
AudioEncoder.swift adds print statements with emoji that fire on every 500th feed() call and on the first 3 AAC frame emissions. These will pollute logs in production builds and slightly impact audio encoding performance on the hot path.

// AudioEncoder.swift
nonisolated(unsafe) private var feedCount = 0
public func feed(_ sampleBuffer: CMSampleBuffer) {
    feedCount += 1
    if feedCount == 1 || feedCount % 500 == 0 {
        print("🔊 AudioEncoder.feed() called #\(feedCount)...")
    }

Recommendation: Remove or gate behind #if DEBUG. Also, nonisolated(unsafe) on feedCount is a data race risk since feed() can be called from multiple threads — use OSAtomicIncrement64 or Mutex instead.

2. Silent audio generator has if true dead code and memory concern
In VideoEncoder.swift, the silent audio generator contains if true { ... } which is clearly leftover from development. More concerning, it creates a new CMAudioFormatDescription and CMBlockBuffer every ~21ms for the entire duration of the stream, which is wasteful.

// Always generate silent audio as fallback
if true {
    // Create a CMSampleBuffer with silent PCM data
    var formatDesc: CMAudioFormatDescription?

Recommendation: Remove the if true guard. Cache the CMAudioFormatDescription and reuse it. Consider only generating silent audio when no real audio source is active, rather than always.

3. YouTube OAuth scope broadened significantly without justification
The YouTube OAuth scope changed from youtube.readonly + youtube.upload + youtube.force-ssl to just youtube + youtube.force-ssl. The youtube scope grants full read/write/delete access to the user's YouTube account. This is overly permissive.

// Before: youtube.readonly youtube.upload youtube.force-ssl
// After:  youtube youtube.force-ssl

Recommendation: Use the minimum required scopes. For live streaming, you need youtube.force-ssl (which covers live broadcasts) — determine the exact minimal set rather than granting full account access.

---

🟡 Major

4. PlatformConfig.transitionTask stores a Task in a struct — value semantics issue
StreamViewModel.PlatformConfig is a struct containing a Task<Void, Never>?. Since structs use value semantics, copying a PlatformConfig (e.g., via platformConfigs[.youtube, default: PlatformConfig()]) can silently duplicate or lose the task reference.

struct PlatformConfig {
    var streamKey: String = ""
    var broadcastId: String?
    var transitionTask: Task<Void, Never>?  // ⚠️ Reference type in value type

Recommendation: Either make PlatformConfig a class, or store the transitionTask separately (e.g., var youtubeTransitionTask: Task<Void, Never>? on the view model directly — which you already have as a computed property, so remove it from the struct).

5. @unchecked Sendable on MLXBackend and MLXResponseProvider
Both types use @unchecked Sendable to bypass concurrency checking. MLXBackend uses Mutex which is correct, but MLXResponseProvider has mutable properties (activeRole, voiceLocale, contextInjection) with no synchronization.

public final class MLXResponseProvider: LLMResponseProvider, @unchecked Sendable {
    public var activeRole: AvatarRole = .sidekick     // ⚠️ No synchronization
    public var voiceLocale: VoiceLocale = .english     // ⚠️ No synchronization
    public var contextInjection: String?               // ⚠️ No synchronization

Recommendation: Either make MLXResponseProvider an actor, use Mutex for mutable state, or ensure it's only ever accessed from @MainActor.

6. No error handling for failed RTMP destinations in simulcast
In VideoEncoder.startStreaming(to destinations:), if one destination fails to connect in the withThrowingTaskGroup, the entire simulcast setup throws. For true simulcast, you likely want partial success (e.g., Twitch connects but YouTube fails — you should still stream to Twitch).

try await withThrowingTaskGroup(of: StreamDestination.self) { group in
    for dest in destinations {
        group.addTask { /* connect */ }
    }
    for try await dest in group {  // ⚠️ One failure kills all
        streamDestinations[dest.id] = dest
    }
}

Recommendation: Use TaskGroup instead of ThrowingTaskGroup and handle per-destination errors, allowing partial success with user notification.

7. YouTube broadcast hardcodes "public" privacy status
YouTubeClient.createAndBindBroadcast hardcodes privacyStatus: "public". The StreamInfoFormView has a privacy picker (privacyStatus state), but it's never wired through to the broadcast creation.

"status": ["privacyStatus": "public"]  // ⚠️ Ignores user's selection

Recommendation: Pass the privacy status through StreamViewModel to YouTubeClient.createAndBindBroadcast().

8. Duplicate streaming logic between RecordView and StreamViewModel
Both RecordView.startStreaming() and StreamViewModel.startStreaming() contain nearly identical YouTube broadcast creation, multi-destination setup, and transition logic. This duplication creates a maintenance risk and makes it unclear which code path is actually used.

Recommendation: Consolidate into StreamViewModel as the single source of truth for streaming logic. RecordView should delegate entirely to it.

---

🟢 Minor

9. BackendState uses ~Copyable unnecessarily

private struct BackendState: ~Copyable {

BackendState is only ever used inside a Mutex<BackendState>, which already prevents copies. The ~Copyable annotation is unnecessary and adds complexity. Also, since it holds optional Task and ModelContainer references, it should still work fine as a regular struct in the Mutex.

10. Magic numbers in video bitrate estimation

private var videoBitrate: Int {
    let cores = ProcessInfo.processInfo.activeProcessorCount
    if cores >= 8 { return 4_500_000 }
    if cores >= 4 { return 3_000_000 }
    return 1_500_000
}

These should be documented constants or reference the VideoEncoder's actual auto-detected bitrate.

11. com.apple.security.network.server entitlement added without documentation
The server network entitlement was added to ArkavoCreator.entitlements. This allows the app to listen for incoming connections. While likely needed for the YouTube OAuth local callback server, it should be documented.

12. Inconsistent platform identification strings
Platform identification uses raw strings ("twitch", "youtube") in ChatPanelViewModel.connectedPlatforms and StreamEvent.platform, but enums elsewhere. This creates fragility.

Recommendation: Use the StreamPlatform enum consistently, or at minimum define string constants.

13. Missing scrollContentBackground(.hidden) in PublicistView's TextEditor
PublicistPanelView correctly uses .scrollContentBackground(.hidden) on its TextEditor, but PublicistView.sourceInputSection does not, which will show a default white background on macOS.

14. Hardcoded frame-rate drop threshold in video send task

if elapsed < .milliseconds(Int(targetInterval * 900)) && !frame.isKeyframe {
    continue // Drop frame — too fast
}

The 900 multiplier (90% of target interval) is a magic number that should be documented or made configurable.

15. Several TODO/cleanup items

• The PublicistViewModel.mapContentTypeToAction() maps .thread to .draftPost with a comment "Thread uses draft post with thread-specific prompt" — but no thread-specific prompt exists.
• sentimentLabel in StreamStateContext doesn't handle the exact value 0.3 (it falls through to default: "Positive" since the range 0..<0.3 excludes 0.3 and 0.3..<0.7 starts at 0.3 — actually this is correct, but worth double-checking edge cases).

---

Suggestions

1. Consider splitting this PR: This PR touches three distinct feature areas (MLX AI, simulcast streaming, YouTube Live API). Splitting into 2-3 focused PRs would make review, testing, and potential rollback much safer.

2. Add unit tests for core logic: The PR adds UI tests but no unit tests for critical logic like PublicistPromptBuilder, StreamStateContext.formattedForPrompt(), ModelRegistry.availableModels(), or StreamViewModel.validateInputs(). These are all easily unit-testable.

3. Add a model memory warning: ModelManager auto-loads models on init if cached. On low-memory devices, this could cause issues. Consider adding a memory pressure observer (DispatchSource.makeMemoryPressureSource) to auto-unload.

4. Document the YouTube broadcast lifecycle: The ready → testing → live → complete state machine is non-trivial. A brief architecture doc or code comment explaining the flow and retry logic would help future maintainers.

5. Consider rate limiting the YouTube chat poller: The minimum polling interval is clamped to 5 seconds, but YouTube's API quota is limited. Consider exponential backoff on errors and tracking quota usage.

6. Consolidate animation constants: The PR introduces .spring(response: 0.3, dampingFraction: 0.75) in many places. Consider defining a shared animation constant like Animation.panelSpring.

---

Questions

1. Is the youtube feature flag intentionally enabled (true) in this PR? It was previously false. Is this ready for production, or should it remain gated?

2. What's the expected behavior when MLX model loading fails on a device with insufficient memory? The ModelManager shows an error state, but the Publicist/Producer panels don't provide recovery guidance to the user.

3. Is the silent audio generator needed for all platforms, or only YouTube? The comment says "required by YouTube" but it runs for all RTMP destinations. Consider gating it to only generate when YouTube is a selected destination.

4. Are the Twitch EventSub scopes (moderator:read:followers, channel:read:subscriptions, bits:read) already approved for the app? Adding new OAuth scopes may require Twitch app review.

5. Why was the isCreator gating removed from availableSections? The old code filtered out .patrons for non-creators. The new code relies entirely on feature flags, which means the Patreon section would appear for all users if the flag is enabled.

---

Overall Recommendation

Request Changes 🔄

The architecture and feature design are strong, but the PR should address the critical issues (debug logging in production audio path, silent audio generator cleanup, overly broad YouTube OAuth scope) and the major issue of @unchecked Sendable without synchronization before merging. The duplicate streaming logic between RecordView and StreamViewModel should also be consolidated to prevent maintenance issues.

I'd also strongly recommend splitting this into smaller PRs if timeline permits — the blast radius of a 4900-line PR across core streaming infrastructure is high.

[sonarqubecloud]

Quality Gate passed

Issues
24 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.6% Duplication on New Code

See analysis details on SonarQube Cloud