Files

T

Claude 28358b4141 refactor(nests): extract ActiveSubscription + plan deferred manager refactor (Audit-9, Audit-14)

Audit-9: NestViewModel.kt is 2112 lines and growing, ~1000 of which
are subscription-lifecycle state machine concerns intertwined with
the room-level public API. Pulling out the full
`NestSubscriptionManager` is a multi-week refactor with subtle
coupling (catalog readiness affects spinner state, mute has effective
+ per-speaker flavours, expiry jobs need the parent scope) and
warrants its own focused review pass.

For now, take the small tractable subset:

  - Extract `ActiveSubscription` from a `private inner class` in
    NestViewModel to its own file as `internal class
    ActiveSubscription` in the same package. The class is purely
    state-holding (handle, roomPlayer, player, isPlaying); zero VM
    coupling beyond the slot map's value type. Same visibility for
    NestViewModel callers; one less private helper class buried
    1500 lines into NestViewModel.kt.

  - File `commons/plans/2026-05-06-nest-subscription-manager-extraction.md`
    documents the deferred full extraction: target shape, state
    that moves, methods that move, what stays in VM, why deferred,
    when to land. Picks up the next person who opens NestViewModel.kt
    rather than leaving them to re-derive the rationale.

Audit-14: T11.3 (stream priority for moq-lite group uni streams) was
deferred from the T11 commit (drop bestEffort=true) because the
:quic-side change touches the writer's hot path and warrants its own
review pass. New file `nestsClient/plans/2026-05-06-stream-priority-followup.md`
spells out:

  - Why: bestEffort=true was incidentally biasing drain order toward
    newer groups; without it, the writer's round-robin order can
    serve a stale group when a fresh one is more useful.
  - Target shape: `QuicStream.priority` field, sortedByDescending
    in the writer's send-frame loop, `WebTransportWriteStream.setPriority`
    pass-through, `MoqLiteSession.openGroupStream` calls
    setPriority(sequence). With code sketches.
  - Test: pin iteration order via the writer's emitted-frames tape.
  - Risk profile: starvation, perf cost of per-pass sort, compat.
  - When to land: after interop verification stabilises.

No code changes in this commit beyond the ActiveSubscription move.

https://claude.ai/code/session_014JfZJHSTvyYYWJbC9VbB47

2026-05-06 20:04:56 +00:00

4.1 KiB

Raw Blame History

Extract `NestSubscriptionManager` from `NestViewModel`

Status: deferred — flagged in the audit pass, not landed yet.

Why

NestViewModel.kt is 2112 lines and growing. ~1000 of those lines are the per-speaker subscription-lifecycle state machine: open / close / reconcile / catalog-fetch / decoder-await / level-tap / mute-routing / hush. The rest is room-level public API (connect, disconnect, broadcast, presence, reactions, focus / network observers, UI state).

These two concerns are coupled by shared mutable state but do not share a responsibility. The audit-9 finding flagged it as the single biggest SRP breach in the touched code.

We extracted ActiveSubscription into its own file (ActiveSubscription.kt) in commit <TBD> as a stepping stone — the deferred extraction proper is the orchestration class.

Target shape

internal class NestSubscriptionManager(
    private val viewModelScope: CoroutineScope,
    private val signer: NostrSigner,
    private val decoderFactory: (channelCount: Int, sampleRate: Int) -> OpusDecoder,
    private val playerFactory: (channelCount: Int, sampleRate: Int) -> AudioPlayer,
    private val onActiveSpeakersChanged: (Set<String>) -> Unit,
    private val onSpeakerActivity: (String) -> Unit,
    private val onAudioLevel: (String, Float) -> Unit,
    private val onConnectingSpeakerChanged: (pubkey: String, connecting: Boolean) -> Unit,
    private val effectiveListenMuted: () -> Boolean,
    private val locallyHushed: () -> Set<String>,
) {
    val speakerCatalogs: StateFlow<Map<String, RoomSpeakerCatalog>>
    val audioLevels: StateFlow<Map<String, Float>>

    fun bind(listener: NestsListener)
    fun unbind()  // cancel everything; release native resources
    fun updateSpeakers(requested: Set<String>)
    fun applyEffectiveMute()
    fun setLocalHushed(pubkey: String, hushed: Boolean)
}

State that moves

From NestViewModel:

activeSubscriptions: MutableMap<String, ActiveSubscription>
catalogJobs: MutableMap<String, Job>
_speakerCatalogs: MutableStateFlow<Map<String, RoomSpeakerCatalog>>
requestedSpeakers: Set<String>
speakingExpiryJobs: MutableMap<String, Job> (for onSpeakerActivity debounce)
_audioLevels if separable

Methods that move

reconcileSubscriptions
openSubscription (~150 lines)
closeSubscription
fetchSpeakerCatalog
awaitAudioPipelineConfig
onSpeakerActivity debounce timer
onAudioLevel coalescing
applyEffectiveListenMute's subscription-touching half
setLocalHushed's player.setVolume half
publishActiveSpeakers

What stays in `NestViewModel`

Public lifecycle (connect, disconnect, onCleared)
Connection state machine (ConnectionUiState)
Broadcast state (broadcast / _uiState.broadcast)
Presence aggregation (kind 10312 events)
Reactions
Focus / network observers
The NestUiState composition itself

NestViewModel calls manager.bind(listener) on each fresh listener and manager.unbind() on disconnect / cleanup. updateSpeakers and mute / hush propagate through to the manager.

Why deferred

1000-line code move across ~10 methods + 5 state fields
Subtle coupling: catalog readiness affects spinner state, mute state has effective + per-speaker flavours, expiry jobs need parent scope, closed flag is currently a single VM-wide flag
Test surface: NestViewModelTest exercises subscription paths through the VM today; would need to either keep that surface or add a NestSubscriptionManagerTest.

The extraction has a clean contract (callbacks for the bits that remain VM-side) but landing it without behavioural drift wants a focused review pass plus its own dedicated test rebuild — bigger than fits in the current audit-pass.

When to land

After:

The catalog-driven decoder reconfig (T3) has run in production for long enough that the awaitAudioPipelineConfig pattern is proven against real publishers.
Any pending subscription-lifecycle bug fixes (e.g. the audit's earlier "boundary-rebuild dangling decoder" path) are settled — moving them mid-fix risks introducing regressions.

Track as a follow-up issue rather than a near-term must-do.

4.1 KiB Raw Blame History

Extract NestSubscriptionManager from NestViewModel

Why