--- title: Playback queue overhaul (OD-7458) tags: [work, android, exoplayer, refactor, testing, progress] created: 2026-06-10 updated: 2026-06-10 --- OD-7458 — the multi-PR refactor of Noice Android's playback queue. Goals: add Spotify/YouTube-style **previous playback**, make queue logic **unit-testable** without Android/ExoPlayer deps, and **delete the `object QueueManager` god-singleton**. Shipped on `dev/OD-7458/queue_overhaul`, 21 commits, 73 files changed, between 2026-05-20 and 2026-06-10. ### LOC delta (honest read) | Slice | Added | Deleted | Net | |---|---|---|---| | Raw diff (everything) | 4657 | 1063 | +3594 | | Tests | 1733 | 2 | +1731 | | Docs (`*.md`) | 895 | 0 | +895 | | Production code (incl. blanks + comments) | 2029 | 1061 | +968 | | **Production code (blanks + comments stripped)** | **1350** | **861** | **+489** | Net **+489 executable LOC** for the feature + refactor. Roughly half the headline diff is tests, a quarter is docs, and ~14% is real production growth. The 861 deleted code-lines were the worst-coupled bits — god-object methods, observer chain, inline `initQueue` rebuild, `Thread.sleep` / `System.gc` hacks. ## Context The legacy queue lived in `object QueueManager` — a Kotlin singleton holding `ObservableArrayList`, with queue mutation, persistence, ExoPlayer media-source rebuild, content-version detection, and Cast sync all interleaved. Self-rated 4/10: - Untestable (`object` + `BaseApplication` + `EventBus` + `PrefUtils` + `Log` couplings). - Race conditions — `OnListChangedCallback` fired on the UI thread while PlayerManager iterated the same list. - Lost work — items popped from index 0 went to `/dev/null`. **No previous-track support**: `setUsePreviousAction(false)`, `onSkipToPrevious()` commented out at `PlayerManager.kt:1425`. - Wasteful rebuild — every drag-reorder re-built the entire `ConcatenatingMediaSource`, destroying the prepared source ExoPlayer was actively playing. - Silent failure — `PrefUtils.getQueueList()` returned `null` on parse failure → queue silently lost. 123 call sites across 38 files touched `QueueManager` directly. 0 tests covered queue logic. This connects to a broader thread: doing the overhaul on legacy code is what triggered the [[kotzilla-noice-di-synthesis|"rewrite Noice in Compose"]] musing in [[2026-W21]]. ## Target architecture A pure-Kotlin `QueueEngine` owns all queue logic, exposes a `StateFlow`, and has zero Android deps. Three Hilt `@Singleton` classes wrap it; one bridge talks to ExoPlayer. ``` ┌──────────────────────────────────────────────┐ │ QueueEngine — pure Kotlin (@Singleton) │ │ StateFlow, mutation methods │ └──────────────────────────────────────────────┘ ▲ ┌──────────────────────────┼──────────────────────────┐ │ │ │ ┌──────────────┐ ┌────────────────┐ ┌──────────────────┐ │ QueueReader │ │ QueueController│ │ QueuePayloadStore│ │ (read-only) │ │ (mutations + │ │ (HashMap, │ │ │ │ │ │ @Synchronized) │ └──────────────┘ └────────────────┘ └──────────────────┘ │ ▼ ┌────────────────┐ │ QueueExoBridge │ observes snapshot, │ │ reconciles 2-slot │ │ window via key-based │ │ move (preserves prepared │ │ MediaSource, no glitch) └────────────────┘ ``` ### Components at a glance | Class | What it owns | Sample API | |---|---|---| | `QueueEngine` | All queue state mutations, history, snapshot emission. Pure Kotlin, no Android deps. | `advance(positionMs)`, `previous(positionMs)`, `addToManualQueue`, `replaceAutomaticQueue`, `applyContentEdits`, `restore`, `clear` | | `QueueReader` | Pure reads. Safe to call from any thread. Narrow surface — callers that only display the queue can't accidentally mutate it. | `queue`, `getMediaData(id)`, `getPlayingData()`, `getFirstMedia()`, `getPayload(item)`, `isPreviousAvailable()`, `isHandledByQueue()` | | `QueueController` | Mutations + side effects (`PrefUtils` persistence, `PlayerEvent` dispatch, `EventBus.QUEUE_UPDATE`, catalog API fetches via `ChannelPodcastApiRepository`). 429 LOC — fattest class in the layer. | `initializeQueue`, `onAutoAdvance(positionMs)`, `addToManualQueue`, `alterAutomaticQueue`, `swipe`, `handleQueueItemClick`, `previous(positionMs)`, `updateQueueData`, `clearAndNotify`, `reFetchQueue` | | `QueuePayloadStore` | Thread-safe `HashMap`. Both Reader and Controller depend on it; neither has hidden access to the other's surface. | `register`, `put`, `get`, `containsKey`, `clear` (`@Synchronized` writes) | | `QueueExoBridge` | Observes engine snapshot, reconciles ExoPlayer's 2-slot playlist window via key-based move (preserves prepared `MediaSource`). Owns HLS URL normalization upfront. | `attach(host, scope)` — internal `reconcile()` runs on snapshot emit | | `QueueBridgeHost` | Pure-Kotlin interface the bridge writes to. `PlayerManager` implements it for production; tests use a list-backed `FakeHost`. | `playlistSize()`, `keyAt(i)`, `needsReload(i, item)`, `removeAt(i)`, `moveSlot(from, to)`, `insertAt(i, item)` | Plus two small helpers: - `QueueAccessors.kt` (19 LOC) — top-level `queueReader` / `queueController` for non-Hilt callers; resolves via `BaseApplication.commonEntryPoint`. - `QueueItemMapper.kt` (66 LOC) + `QueueModels.kt` (57 LOC) — payload ↔ `QueueItem` mapping and snapshot data classes. - `FinishedThreshold.kt` (69 LOC, lives in `noice.app.player`, not `queue/`) — pure helper for the "is this item effectively finished?" rule (30s remaining OR 90% played). Used by `previous()` to decide resume-at-0 vs resume-at-last-position, and by analytics. ### Why three classes, not two The original plan said `QueueReader` + `QueueController`. Shipped with a third class, `QueuePayloadStore`, holding the shared `HashMap`. - Put it on Controller → Reader would have to inject Controller → narrow read surface transitively pulls in wide mutation surface. - Put it on Reader → Reader has to expose `register / clear / put` → not read-only anymore. - Extract it → Reader gets `get / containsKey`. Controller gets full write API. Neither has hidden access to the other. Cost: ~40 LOC. ## Previous-playback semantics On `previous()`: 1. If current playback position > **3 seconds** → seek to 0 (don't change item). 2. Else if `history` non-empty → push current back to `upcoming.first`, pop `history.last` → current. 3. Else → seek to 0. Rules: - **History cap: 50 items.** - **Resumes at last position** (captured at the moment of `advance()` via `prevData.timeElapsed` → stored on `QueueItem.playbackPositionMs`), not at 0. Mirrors podcast-listener expectation: "go back" means "back to where I was." - **Finished-threshold rule** (30s remaining OR 90% played) — if the popped item was past it, resume at 0 instead of last position. - **Hybrid persistence model** (YouTube/Spotify mix): history survives auto-advance and in-queue navigation, **resets on explicit "play this content"** (anywhere `playFirstItem=true` is set — content detail, catalog page, search). Final decision after a flip-flop: pure YouTube-style felt wrong in QA because previous walked into a totally different prior session. - **Per-content-type policy:** enabled for podcast / audiobook / music; disabled for radio / livestream. - **Content edits while in history:** `applyContentEdits` detects `updatedAt` change for a history item → drops it from history. - History persists across app restarts (schema v2 with v1→v2 migration). ## Testing strategy (TDD, Option B) Did **not** characterize current `QueueManager` behavior — tests against it would be integration tests pretending to be unit tests. Instead wrote tests against the **target `QueueEngine` interface**. - Red phase landed first (commit `d97a41fa4`): 45 failing tests + `TODO()` interface stubs. Contract documented in code. - Green phase (`76642833c`): `QueueEngineImpl` greens the 39 non-`previous()` tests. - PR 3 (`9e879682a`): history + `previous()` greens the remaining 6. - PR 4 (hardening): property tests, integration tests, finished-threshold extraction. **94 `@Test` methods** by the end (property layer runs many randomized cases per method): | Layer | `@Test` methods | Style | |---|---|---| | `QueueEngine` | 45 | Behavior-by-example | | Engine invariants | 3 | Randomized stateful property test + 2 scoped invariants, covers 11 named invariants (**caught 3 latent bugs**) | | `QueueExoBridge` | 10 | Reconciliation against a list-backed `FakeHost` | | `FinishedThreshold` | 14 | Pure math, boundary cases | | `QueueController` | 22 | Orchestration sequences, request routing, edge cases | All run in pure JVM (~50 ms total). The bridge is testable because `QueueBridgeHost` is a pure-Kotlin interface (`playlistSize / keyAt / needsReload / removeAt / moveSlot / insertAt`); `PlayerManager` implements it for production; tests use a `FakeHost`. ## Bridge: move-instead-of-rebuild The bridge maintains a 2-slot window `[current, next]` on the ExoPlayer playlist. On snapshot change it does key-based identity reconciliation: if "new-current" is already at slot 1, MOVE slot 1 → 0 instead of rebuilding the source. This preserves the prepared `MediaSource` ExoPlayer was actively playing — kills the audible glitch and "next briefly disabled" bug from the old `initQueue()` rebuild path. HLS URL normalization moved upfront into `QueueExoBridge.needsReload`, deleting the post-hoc `normalizeHlsUrl()` cleanup at `PlayerManager.kt:1531`. ## What got deleted - `object QueueManager` (584 lines) — gone. - `QueueChangeObserver` (the `OnListChangedCallback` chain) — gone. - `Thread.sleep(50)` + `System.gc()` hacks for edited-content reset — gone (engine handles it synchronously via `applyContentEdits()`). - `PlayerManager.initQueue()` (~120 LOC of inline rebuild) — reduced to a one-line bridge attach. - The "demolish syncMirror" commit (`e955d3438`) made the queue a derived snapshot view instead of a maintained mirror — eliminated a whole class of consistency bugs. ## What still looks legacy (intentionally) - `EventBus.post(QUEUE_UPDATE)` is still how UI fragments learn the queue changed. Long-term fix: `StateFlow>` on `QueueReader` + DiffUtil. Deferred until **Compose adoption** — bigger return on investment then. - `notifyDataSetChanged()` in queue adapters. Same path. - `ConcatenatingMediaSource` (deprecated in Media3 1.3.0+). The bridge already talks to the pure `QueueBridgeHost` interface, so swap to `player.addMediaItem` is a one-class change. Top-level `queueReader` / `queueController` accessors in `QueueAccessors.kt` resolve via `BaseApplication.commonEntryPoint` — service-locator shortcut so the big-bang migration was mechanical. Promotion to per-class `@Inject` is tracked as future work (~1 day, no user-visible benefit). ## How execution diverged from the plan - **Big-bang QueueManager deletion** (commit `d459a6a33`) instead of the originally-planned 6-phase rollout. Trade-off: phased gives per-PR bisectability; big-bang ships clean state immediately. Chose big-bang because the migration was mechanical (replace `QueueManager.foo()` with `queueReader.foo()` / `queueController.foo()`) — small bug surface, 41 call sites in one reviewable diff. - **Three classes, not two** — see "Why three classes, not two" above. - **History reset on explicit play** flip-flop (2026-05-21) — went pure-YouTube, reverted to hybrid after QA showed the cross-session walk-back felt wrong. ## Commit timeline (origin/develop..dev/OD-7458/queue_overhaul) | Date | Commit | Story | |---|---|---| | 2026-05-20 | `d97a41fa4` | Red-phase baseline: 45 failing tests + `QueueEngine` interface | | 2026-05-20 | `76642833c` | Green phase sans `previous()` — `QueueEngineImpl` implemented | | 2026-05-20 | `8b264cca8` | `QueueManager` wired as facade over `QueueEngine` | | 2026-05-20 | `66f79b82a` | Facade QA fixes: playing panel, next button, drag-reorder | | 2026-05-21 | `9e879682a` | **PR 3** — history + `previous()` end-to-end | | 2026-05-22 | `568b6946b` | Notification + MediaSession previous routed through engine | | 2026-05-22 | `613bd4602` | **PR 2** — `QueueExoBridge` extracted from `PlayerManager` | | 2026-05-25 | `4af50a32b` | Harden persistence + drop legacy queue observer | | 2026-05-26 | `e955d3438` | Demolish `syncMirror`; queue becomes a derived snapshot view | | 2026-05-28 | `3745a9c2b` | Hardening QA fixes: next autoplay, catalog leak, previous edge cases | | 2026-05-29 | `37cf3a4c2` | Previous resumes at last position; restart if finished | | 2026-05-30 | `dfaec2e69` | Property tests for engine invariants + 3 bug fixes | | 2026-06-01 | `b67661dfa` | Bridge integration tests with list-backed fake host | | 2026-06-01 | `3398035ce` | Extract `FinishedThreshold` helper + 12 unit tests | | 2026-06-02 | `3f9df80f7` | Prune dead code in `QueueManager` / `PlayerManager` | | 2026-06-02 | `d459a6a33` | **Delete `object QueueManager`; split into Reader + Controller + PayloadStore** | | 2026-06-02 | `0070f38bc` | Document the deletion outcome | | 2026-06-02 | `82ad63844` | Add before/after architecture comparison doc | | 2026-06-10 | `a767edeb7` | `QueueControllerTest` — 22 orchestration tests | | 2026-06-10 | `b35ccfe76` | Refresh docs after task #6 ships | (Plus one earlier facade-wire commit. Full bodies in `raw/work/queue-overhaul/commits.md`.) ## Quality rating (self-assessment) | Dimension | Before | After | |---|---|---| | Testability of pure layers | 1/10 | 9/10 | | Testability of orchestration | 1/10 | 8/10 | | Separation of concerns | 2/10 | 6.5/10 (Controller bloated to 429 LOC during hardening) | | Race safety | 3/10 | 8/10 | | Feature support (previous, history, resume) | 1/10 | 9/10 | | Observability | 4/10 | 7/10 | | Hilt integration | 0/10 | 6/10 (top-level accessors still used) | | UI reactivity | 3/10 | 3/10 (unchanged — gated on Compose) | | **Overall** | **~4/10** | **~7.5/10** | Named follow-ups holding back the score: UI reactivity (needs Compose adoption), per-class `@Inject` migration (mechanical), further Controller split into Persister + AutoFetcher (now 429 LOC). ## Token-cost A/B — preliminary internal measurement Ran the 5-task comprehension protocol (see `raw/research/ai-code-quality/token-experiment/tasks.md`) against both branches of `noice-android`: - **Branch A** (control): `origin/develop` — legacy `object QueueManager`. - **Branch B** (treatment): `dev/OD-7458/queue_overhaul` — refactored Engine / Reader / Controller / PayloadStore + bridge. Identical prompts, Claude Code default model, one trial per branch. | Metric | Legacy | Refactored | Delta | |---|---:|---:|---:| | **Total cost** | $2.12 | **$0.95** | **−55%** | | Wall-clock time | 8m 34s | 3m 53s | −55% | | API time | 11m 42s | 6m 29s | −45% | | Haiku output tokens | 40.3k | 20.5k | −49% | | Haiku cache reads | 9.1M | 3.8M | −58% | | Opus output tokens | 9.6k | 2.1k | −78% | ### What the data says Same prompt, same agent, same model defaults, **same agent topology** (5 parallel `Explore` subagents per run, one per task). Only the code differs. The refactored branch cost **2.23× less** to produce equivalent answers. The Haiku cache-read drop (9.1M → 3.8M, **2.4× fewer reads**) is the cleanest input-side signal: less context had to be reloaded across tool calls to reach equivalent understanding. The legacy `object QueueManager` intersects with nearly every queue-touching file, so any task that touches the queue layer pulls a wide swath of code into context. The refactored Engine/Reader/Controller/PayloadStore split lets each subagent focus on one named class. ### Honest caveats - **n = 1 per branch.** Single trial. Noise contribution unknown — a second pair would firm up the numbers. - **Model-mix differed.** Opus did more output on the legacy run (9.6k vs 2.1k). Some of the cost delta is model selection — Claude Code escalates to Opus when Haiku struggles, which is itself a code-quality signal but not pure token volume. - **Output-shape differed.** The legacy answer is more detailed walkthrough-style; the refactored answer is more concise. A quick blind quality cross-check would confirm both equally answer the same questions. - **Raw files**: `raw/work/queue-overhaul/[before] queue overhaul agentic cost test.md` and `[after] queue overhaul agentic cost test.md`. This is, to my knowledge, the only published clean A/B (same task, varied code quality on the same codebase, identical agent topology) for agentic LLM token cost. Even with n=1, it's the closest empirical answer Q3 has gotten — and it points in the direction the thesis predicted, hard. ## Related - [[kotzilla-noice-di-synthesis]] — overhaul reinforced the "rewrite in Compose" thread - [[hilt-to-koin-migration]] — DI direction for the broader app - [[2026-W21]], [[2026-W22]] — weekly notes during the overhaul - Raw material: `raw/work/queue-overhaul/` (4 docs + commits + diff summary)