Sendspin compliance update

[teancom]

Updated the library to be a conformant Sendspin client. Added tests and documentation. Passes all of the tests in the conformance repo and extensively tested manually with a Bruce Springsteen song. It's... a lot! I had Claude write up a summary of the changes across the branch:

This branch brings SendspinKit from a rough prototype to a spec-compliant, production-hardened client library. The work spans the entire stack — audio pipeline, sync engine, networking, public API, and developer ergonomics. The audio pipeline was rebuilt from the ground up. Playback was broken at the start of this branch; it now works end-to-end with FLAC, Opus, and PCM codecs. A Kalman time filter (ported from C++) drives sync correction, a perceptual volume curve shapes output, and CPU wake-ups were cut 63% through smart sleep and dead-loop removal. The AudioPlayer was hardened with lock-protected state via OSAllocatedUnfairLock, and the decoder got safety fixes for Opus overflow and interleaved access. Hardware vs. software volume control is now a first-class concept with VolumeMode, and an AudioProcessCallback allows consumers to tap the raw audio stream for visualization or effects.

On the protocol and networking side, the client supports mDNS advertisement for server-initiated connections, alongside direct client-initiated connections, supports seamless mid-stream format switching via stream/request-format, and implements controller role commands (play/pause/next/prev) and artwork retrieval. The transport layer was overhauled to fix data races, encoding waste, and disconnect semantics. fputs logging was replaced with structured os.Logger across every subsystem.

The public API went through a full ergonomics pass — typed errors, Sendable conformances, continuous server discovery, and a clean separation between SendspinClient and the roles it can assume. Internally, nearly every subsystem was hardened: magic numbers replaced with named constants, preconditions added, dead code removed (guided by a Periphery scan), and stale comments cleaned up. SwiftFormat and SwiftLint were integrated and enforced. The branch also adds a DocC documentation catalog, a MockTransport for testing, and integration tests for SendspinClient.

[balloob]

SendspinKit PR #6 — Comprehensive Architecture Review

I read every source file in this PR (all 87 changed files, 10K additions). This is genuinely impressive work — taking a rough prototype to a spec-compliant, production-hardened client library. The architecture is fundamentally sound, the concurrency model is well-thought-out, and the audio pipeline is correctly layered. The inline comments cover the most important issues; this summary provides the full picture.

---

What's Done Well

Audio pipeline architecture — The layering (AudioScheduler → AudioPlayer → PCMRingBuffer → AudioQueue callback) is clean. The separation between the async scheduling actor and the real-time C callback is the right design. The LockedState pattern with OSAllocatedUnfairLock for audio thread synchronization is textbook correct.

Kalman time filter — Faithful port with full 2D covariance propagation, adaptive forgetting, drift SNR gating, and RTT floor. The math checks out. The TimeFilterSnapshot pattern for crossing actor→audio-thread boundaries without actor hops is elegant.

Nullable<T> for JSON delta merges — Clean solution to the absent-vs-null-vs-value problem. The Encodable guard that throws for .absent prevents misuse. The ServerMetadataState usage pattern is correct.

Transport protocol abstraction — The inbound/outbound split (WebSocketTransport vs NWWebSocketTransport) with a shared SendspinTransport protocol is well-motivated. The common interface covers exactly the right surface area.

VersionedRole design — ExpressibleByStringLiteral for Swift-side ergonomics, strict Decodable for wire format. Static constants for spec-defined roles. Clean.

Test quality (where it exists) — PCMRingBufferTests is exemplary. AudioPipelineIntegrationTests are genuine end-to-end pipeline tests. SendspinTimeFilterTests demonstrates rigorous numerical verification. ClientIntegrationTests test 7 (raw audio chunks before stream/start) is an excellent regression test for a specific race condition.

DocC catalog — Good investment for a library. The articles cover discovery, events, and the audio pipeline.

---

Critical Issues (2)

#	Issue	Location
C1	precondition in public initializers crashes host app — PlayerConfiguration, ArtworkConfiguration, AudioFormatSpec, ArtworkChannel, and SendspinClient.init all use precondition/preconditionFailure for input validation. A library must never trap for invalid input. Use throwing initializers.	Multiple files (see inline)
C2	precondition on audio thread in advanceCursor() — Trapping from the AudioQueue callback kills the host process. Should be guard+return or debug-only assert.	AudioPlayer.swift:96

Major Issues (12)

#	Issue	Location
M1	Try-all-types JSON decoding chain — handleTextMessage tries decoding every message type sequentially. Should extract "type" first (already done!) then switch-decode in one pass.	SendspinClient+MessageHandling.swift:24-43
M2	HardwareVolumeControl missing perceptual gain curve — Software volume applies pow(v, 1.5), hardware volume passes raw linear. Volume 50% sounds different between modes.	VolumeController.swift:79
M3	public internal(set) on ~6 state properties — Overly wide mutation surface. Should be public private(set) with internal setter methods for extensions. No state machine validation on transitions.	SendspinClient.swift:27-62
M4	AudioScheduler.queue uses O(n) Array operations — insert(at:) and removeFirst() shift the entire array. With 2500+ chunks buffered, this is significant. Use Deque or index-tracking.	AudioScheduler.swift:93,169,179
M5	WebSocket connect() continuation leak — If Starscream never calls back or the task is cancelled, the stored continuation is never resumed. Needs withTaskCancellationHandler.	WebSocketTransport.swift:140-143
M6	sendCommand exposes invalid parameter combinations — sendCommand(.play, volume: 50) compiles but is nonsensical. Make it internal; the typed convenience methods are the correct API.	SendspinClient+Commands.swift:141
M7	PlaybackProgress.currentPositionMs(at:) unusable — Requires server clock domain timestamp but no public method to obtain one. Dead API for consumers.	ClientTypes.swift:147
M8	No recovery path from ConnectionState.error — Client is stuck in degraded state. No docs or API for recovery.	ConnectionState.swift:37-42
M9	PlayerStateObject validation bypassed by synthesized Decodable — Preconditions in manual init are never called during JSON decoding. Malformed server data accepted silently.	SendspinMessage.swift:255-258
M10	Per-call heap allocation in Opus/PCM decoders — OpusDecoder.decode() and PCMDecoder.decode24Bit() allocate on every call. FLACDecoder correctly reuses buffers — others should too.	AudioDecoder.swift:60,135
M11	ServerDiscovery.deinit accesses actor-isolated state — pendingResolves and resolveTimeoutTasks accessed from non-isolated deinit. Formal data race in Swift 6 strict concurrency.	ServerDiscovery.swift:304-313
M12	ServerDiscovery has zero test coverage — Resolve logic, IPv6 formatting, TXT record parsing, error retry, TerminatedError, stream lifecycle all untested.	Missing ServerDiscoveryTests.swift

Minor Issues (15)

#	Issue	Location
m1	Three fragmented error types (TerminatedError, SendspinClientError, StreamingError) with no shared namespace	SendspinKit.swift, ClientTypes.swift, ConnectionState.swift
m2	ClientEvent.streamCleared discards role information — consumers can't tell which stream cleared	ClientTypes.swift:60
m3	AudioQueue callback lock scope is very wide (~85ms at 48kHz) — blocks actor's playPCM writes	AudioPlayer.swift:456-563
m4	swapDecoder doesn't update currentFormat — stale value during format transition window	AudioPlayer.swift:338-346
m5	discoverServers(timeout:) task-group logic is subtly fragile with race-dependent result ordering	SendspinClient.swift:145-171
m6	Volume API uses Int(0-100) externally but Float(0-1) internally — inconsistent mental model, undocumented	SendspinClient.swift:633
m7	setVolume/setMute throw for .notConnected but silently swallow server notification failures — asymmetric error behavior	SendspinClient.swift:638,656
m8	discoveries.values yields unstable ordering in servers stream — could cause unnecessary UI reloads	ServerDiscovery.swift:291
m9	Two SendspinClientTests are no-ops that assert nothing beyond initialization	SendspinClientTests.swift:28-72
m10	AudioProcessCallbackTests all rely on 500ms Task.sleep — flaky on CI without audio hardware	AudioProcessCallbackTests.swift
m11	ClientAdvertiserTests has no tests for connection acceptance, isTerminated, or TerminatedError	ClientAdvertiserTests.swift
m12	Convenience controller methods don't document what happens if the server doesn't support the command	SendspinClient+Commands.swift:148-229
m13	currentVolume resets to hardcoded 100 on disconnect rather than the initial configured value	SendspinClient.swift:297
m14	disconnect() goodbye delivery is best-effort (try?) but not documented as such	SendspinClient.swift:268
m15	In-range but unrecognized binary type IDs (5-7, 17-23) silently dropped with no log	BinaryMessage.swift:57

---

Architecture Assessment

Overall: Strong foundation, needs consumer-facing polish

The internal architecture is solid — the audio pipeline, clock sync, and transport layers are well-separated with clean abstractions. The concurrency model is correct: actors for async state, OSAllocatedUnfairLock for the audio thread, nonisolated let for cross-boundary data, TimeFilterSnapshot for lock-free reads.

Decoupling

The major coupling concern is SendspinClient itself — at ~700 lines across 3 files, it's the orchestrator and necessarily touches everything. The extension split (+Commands, +MessageHandling) is the right organizational approach. The internal (not private) properties for extension access is pragmatic but should be tightened to private(set) with setter methods.

The audio pipeline is well-decoupled: AudioScheduler knows nothing about codecs, AudioPlayer knows nothing about networking, PCMRingBuffer knows nothing about audio. Each can be tested independently. The ClockSyncProtocol enables mock injection for scheduler tests.

Consumer Ergonomics

This is where the most work remains. The API reads well for the happy path (connect, play, pause), but:

• Error handling is fragmented (3 error types, precondition traps, mixed throw/swallow patterns)
• PlaybackProgress.currentPositionMs(at:) is dead API without a server time accessor
• ConnectionState.error is a trap with no documented exit
• Configuration validation crashes instead of throwing

Top 3 Recommendations

1. Replace all precondition in public APIs with throwing inits. This is the single most impactful change for consumer safety.
2. Switch handleTextMessage to type-first dispatch. Extract the type string (already done), then switch to decode the correct type in one pass. Simpler, faster, no cross-type ambiguity.
3. Expose server time for PlaybackProgress. Either add client.currentServerTimeMicroseconds() or compute it internally — without this, the progress API is unusable for building UI.

---

Generated by Claude Code

[teancom]

Thank you for the feedback! I've (attempted to) address the areas of concern.