Adds "Reproducir carpeta" (sequential) and "Reproducir aleatorio" (Fisher-Yates over the name-sorted order) as folder-scoped playable actions, with auto-advance on track completion and skip next/prev. Isolation from live radio is structural, not disciplinary: the public playMediaItem always clears the local queue on any call, and a new private _encolarCambioFuente is the only path that can advance within it. _cambiarFuente, ControladorReconexion, and the reconnect error path are untouched -- confirmed by a byte-for-byte empty diff on all 4 pre-existing radio/reconnect regression suites, independently re-run before and after (21/21 both times). Handler wiring itself is static-review-only (PluriWaveAudioHandler can't be unit-instantiated); the isolation/advance/race-guard decision logic is extracted into cola_local.dart's pure functions, which are fully unit-tested.
238 lines
13 KiB
Markdown
238 lines
13 KiB
Markdown
# Apply Progress: Android Auto Local Music — Phase 3 (Folder Queue + Shuffle)
|
|
|
|
## Status: DONE — all executable tasks complete in a single pass (single PR, `size:exception` per user's explicit choice)
|
|
|
|
Batch: FIRST and ONLY batch. All phases (0-5) implemented in one pass per
|
|
the user's explicit delivery-strategy choice (single PR with `size:exception`,
|
|
acknowledged as the highest-risk change of the session). Phase 6 items are
|
|
manual follow-up (not executable by this agent) or confirmed not applicable.
|
|
|
|
## Tasks completed: 24 / 26 executable checklist items (Phase 6.1 is
|
|
explicitly manual follow-up per tasks.md; Phase 6.2 confirmed not applicable,
|
|
no code change needed)
|
|
|
|
- Phase 0 (Regression Baseline): 0.1, 0.2 — DONE
|
|
- Phase 1 (Pure Foundations — ColaLocal/decidirAvanceCola/avanceEsValido): 1.1, 1.2, 1.3 — DONE
|
|
- Phase 2 (Shuffle & Collision): 2.1, 2.2, 2.3 — DONE
|
|
- Phase 3 (Handler Isolation Wiring, static-review-only): 3.1-3.8 — DONE
|
|
- Phase 4 (Browsable/Playable Actions): 4.1, 4.2, 4.3 — DONE
|
|
- Phase 5 (Open Question + Final Regression): 5.1-5.5 — DONE
|
|
- Phase 6 (Deviations/Manual Follow-up): 6.1 deferred (manual, `flutter analyze`/`--coverage`/`gen-l10n` not run per strict-TDD scope); 6.2 confirmed N/A (no native/manifest change)
|
|
|
|
## Files changed
|
|
|
|
### New
|
|
- `lib/servicios/cola_local.dart` — `ColaLocal` (immutable queue holder),
|
|
`DecisionAvanceCola` enum, `decidirAvanceCola`, `avanceEsValido`. Pure
|
|
Dart, no `just_audio` import, mirrors the `ControladorReconexion`
|
|
extraction pattern so it is fully unit-testable without instantiating
|
|
`PluriWaveAudioHandler`.
|
|
- `test/servicios/cola_local_test.dart` — 18 tests covering `ColaLocal`
|
|
nav/boundaries, `decidirAvanceCola`'s 3-way gate, `avanceEsValido`'s
|
|
`identical()` guard.
|
|
- `test/servicios/controlador_reconexion_local_test.dart` — 3 tests proving
|
|
the design's open-question resolution: a dead local track retries up to
|
|
the DEFAULT `maxReintentos: 5` via the existing `ControladorReconexion`
|
|
(source-agnostic, no special-casing), fails cleanly on the 6th failure
|
|
(no 6th retry, no hang, no crash). Deliberately kept as a SEPARATE file
|
|
from `servicio_audio_reconnect_test.dart` so that protected suite's exact
|
|
pass count stays byte-identical (see "Regression discipline" below).
|
|
|
|
### Modified
|
|
- `lib/servicios/navegacion_auto.dart` —
|
|
- `pistasEnOrdenNombre`, `mezclarFisherYates`, `pistasEnOrdenAleatorio`
|
|
(Fisher-Yates over the canonical name-sorted order, injected `Random`).
|
|
- Two new `ConstructorArbolAuto` prefixes:
|
|
`carpeta_local_reproducir:`/`carpeta_local_aleatorio:`, both
|
|
PLAYABLE (unlike every other `carpeta_local_*` prefix), with
|
|
predicates + strip-by-length codec, collision-proven at index 14
|
|
against the 3 sibling `carpeta_local_*` prefixes and index 13 against
|
|
`carpeta_local:`.
|
|
- `itemsLocales`' page-0 prepend block extended: "Reproducir
|
|
carpeta"/"Reproducir aleatorio" prepended BEFORE the sort/bucket nav
|
|
entries (per Design ADR-5's explicit ordering), guarded by
|
|
`totalPistas > 0` — absent for a folder with only subfolders.
|
|
- `reproducirCarpetaLocal` (fetches children, filters/orders, calls
|
|
`iniciarCola`; no-op on empty/unresolvable folder) and
|
|
`construirMediaItemColaLocal` (resolves a queue track's content URI +
|
|
MediaItem, reusing `reproducirPistaLocal`'s title derivation).
|
|
- Test file `test/servicios/navegacion_auto_test.dart`: 139 tests total
|
|
(was ~120 before this change) — added new groups for the above, PLUS
|
|
updated 4 pre-existing tests whose exact item counts/positions shifted
|
|
because the new page-0 prepend items are real production behavior
|
|
changes (not a regression — this file is not one of the 4 protected
|
|
suites):
|
|
- "exactamente 50 items": `hasLength(51)` → `hasLength(53)`.
|
|
- "51 a 100 items" pagina0: `hasLength(56)` → `hasLength(58)`.
|
|
- "page-0 mode/bucket prepend" (>150-pistas case): first-5-non-playable
|
|
loop replaced with explicit checks (index 0/1 = the two new playable
|
|
action items, index 2-6 = calidad/buckets).
|
|
- `lib/servicios/servicio_audio.dart` (HIGHEST RISK, static-review-only —
|
|
no handler instantiation possible in unit tests) —
|
|
- New fields: `ColaLocal? _colaLocal`, `bool _avanzandoCola`.
|
|
- `playMediaItem` split: public method now ALWAYS clears
|
|
`_colaLocal`/`_avanzandoCola` FIRST, then delegates to new private
|
|
`_encolarCambioFuente` (body is the previous `playMediaItem`
|
|
implementation, byte-identical).
|
|
- New `_reproducirEntradaCola(NodoLocal, {ColaLocal? colaEsperada})` —
|
|
the ONLY other caller of `_encolarCambioFuente` besides public
|
|
`playMediaItem`. `colaEsperada`, when provided, applies the
|
|
`avanceEsValido` mid-await race guard AFTER resolving the content URI
|
|
and BEFORE playing — functionally identical to the design's advance-flow
|
|
pseudocode, refactored so the guard lives inside the sole resolve+play
|
|
entry point (avoids a second, redundant URI resolve).
|
|
- `_manejarFinPista(ProcessingState)` wired as the FIRST line of the
|
|
`playerStateStream` listener, calling `decidirAvanceCola`; sets the
|
|
`_avanzandoCola` latch synchronously before any await for the
|
|
`avanzar` case; the radio `playing && ready` reset branch gained one
|
|
additional line (`_avanzandoCola = false`) alongside its two existing
|
|
untouched statements.
|
|
- `_desactivarCola()`, `_iniciarColaLocal(List<NodoLocal>)` (the
|
|
`iniciarCola` seam for `reproducirCarpetaLocal`).
|
|
- `stop()` now also calls `_desactivarCola()` — `onTaskRemoved` inherits
|
|
this for free (it calls `stop()`).
|
|
- `skipToNext`/`skipToPrevious` overrides added: no-op when
|
|
`_colaLocal == null`; `skipToNext` past the last track deactivates +
|
|
stops (Design ADR-4, no wraparound); `skipToPrevious` at index 0
|
|
clamps to 0 (restarts the track).
|
|
- `playerStateStream`'s `playbackState.add(...)` gated: `controls`/
|
|
`systemActions`/`androidCompactActionIndices` only add
|
|
skip-previous/skip-next when `_colaLocal != null`; the `null` branch is
|
|
byte-identical to the pre-change list (verified: same elements, same
|
|
order, same compact index).
|
|
- `playFromMediaId` gained a THIRD branch (after `eq_preset:`/`pista:`,
|
|
before the station fallthrough) for the two new action prefixes,
|
|
calling `reproducirCarpetaLocal` with `iniciarCola: _iniciarColaLocal`.
|
|
- Confirmed UNTOUCHED by this change (verified via diff read): `_cambiarFuente`,
|
|
`ControladorReconexion` usage/`_reconexion` field, `_esErrorDeRed`,
|
|
`_gestionarErrorReproduccion`, `_intentarReconexion`, `_reintentarFuente`.
|
|
- `openspec/changes/android-auto-local-music-phase3/tasks.md` — all 24
|
|
executable checklist items marked `[x]` with brief inline notes; 6.1 left
|
|
unchecked (manual follow-up, explicitly out of this agent's scope).
|
|
- `openspec/changes/android-auto-local-music-phase3/design.md` — both Open
|
|
Questions marked `[x]` confirmed, with the apply-time resolution recorded
|
|
inline.
|
|
|
|
## Test results (independently re-confirmed via a single combined run)
|
|
|
|
Combined run of every touched/new test file
|
|
(`cola_local_test.dart` + `navegacion_auto_test.dart` +
|
|
`controlador_reconexion_local_test.dart` + the 4 protected regression
|
|
suites), `flutter test ... --concurrency=1 --timeout=60s`:
|
|
|
|
```
|
|
+181: All tests passed!
|
|
```
|
|
|
|
Per-file breakdown (from the same run, cumulative counters):
|
|
| File | Tests | Pass | Fail |
|
|
|---|---|---|---|
|
|
| `cola_local_test.dart` (new) | 18 | 18 | 0 |
|
|
| `navegacion_auto_test.dart` (modified) | 139 | 139 | 0 |
|
|
| `controlador_reconexion_local_test.dart` (new) | 3 | 3 | 0 |
|
|
| `servicio_audio_reconnect_test.dart` (protected) | 8 | 8 | 0 |
|
|
| `servicio_audio_session_test.dart` (protected) | 5 | 5 | 0 |
|
|
| `servicio_audio_source_switch_test.dart` (protected) | 3 | 3 | 0 |
|
|
| `servicio_audio_eq_reapply_test.dart` (protected) | 5 | 5 | 0 |
|
|
| **Total** | **181** | **181** | **0** |
|
|
|
|
## Regression discipline (Phase 0 baseline vs Phase 5.4 final — the
|
|
non-negotiable gate)
|
|
|
|
Baseline (Phase 0.1, captured BEFORE any code change):
|
|
`servicio_audio_reconnect_test.dart` = 8, `servicio_audio_session_test.dart`
|
|
= 5, `servicio_audio_source_switch_test.dart` = 3,
|
|
`servicio_audio_eq_reapply_test.dart` = 5. Total 21, 0 failures.
|
|
|
|
Final (Phase 5.4, re-run after ALL code changes, same command, same order):
|
|
8 / 5 / 3 / 5 = 21, 0 failures. **EXACT MATCH — same test names, same
|
|
order, same counts, byte-identical.** No regression in any of the 4
|
|
protected suites. Verified twice independently (once right after the
|
|
`servicio_audio.dart` wiring, once as the final Phase 5.4 gate) with
|
|
identical results both times.
|
|
|
|
One deliberate, disclosed exception to "nothing about the 4 protected files
|
|
changes": Phase 5.2 required a "5 failures → agotado, no 6th retry, no
|
|
hang/crash" test using the DEFAULT `maxReintentos: 5` (the existing test at
|
|
`servicio_audio_reconnect_test.dart:115-137` uses an abbreviated
|
|
`maxReintentos: 2`). Rather than add a 9th test into the protected file
|
|
(which would have changed its pass count from 8 to 9 and technically
|
|
violated the "exact pass count" gate), this was added to a NEW,
|
|
purpose-built file (`controlador_reconexion_local_test.dart`) instead,
|
|
testing the same `ControladorReconexion` class (already fully
|
|
unit-testable, source-agnostic by construction). This satisfies task 5.2's
|
|
substance without touching the protected file's contents or count at all.
|
|
|
|
## Diff size
|
|
|
|
Estimated: prod ~470 lines (`cola_local.dart` ~100 new,
|
|
`navegacion_auto.dart` ~180 added, `servicio_audio.dart` ~190 added/changed
|
|
net of the split), tests ~950 lines across 3 new/modified test files. Total
|
|
~1400+ lines — over the original ~850-950 estimate in tasks.md's Review
|
|
Workload Forecast, primarily because of the number of existing
|
|
`navegacion_auto_test.dart` assertions that needed explicit updates once
|
|
the page-0 prepend behavior changed, plus the dedicated open-question
|
|
regression file. Delivered as a single PR with `size:exception` per the
|
|
user's explicit, informed choice this session (told this was the
|
|
highest-risk change of the day, chose to proceed as one PR anyway).
|
|
|
|
## Deviations from design (disclosed)
|
|
|
|
1. **Mid-await race guard placement**: design's ADR-3 pseudocode describes
|
|
the `avanceEsValido` guard as gating whether `_reproducirEntradaCola`
|
|
gets called at all (guard, THEN call). Implemented instead as an
|
|
optional `colaEsperada` parameter checked INSIDE
|
|
`_reproducirEntradaCola`, immediately after its own URI resolve and
|
|
before calling `_encolarCambioFuente`. This is functionally identical
|
|
(same causal order: capture → await → guard → play) and avoids a
|
|
redundant second `uriContenidoDePista` channel call, while preserving
|
|
task 3.2's stricter acceptance criterion ("`_reproducirEntradaCola` is
|
|
the ONLY other caller of `_encolarCambioFuente`").
|
|
2. **Resolve-failure handling not explicit in design's happy-path
|
|
pseudocode**: when an auto-advance's `construirMediaItemColaLocal`
|
|
resolves to `null` (dead/moved/permission-revoked local file, distinct
|
|
from a `PlayerException` during actual playback), the queue is
|
|
deactivated and stopped rather than left with a stuck `_avanzandoCola`
|
|
latch. This is a defensive addition consistent with ADR-4's "no
|
|
inconsistent state" principle, not explicitly scripted in the design's
|
|
pseudocode.
|
|
3. **`androidCompactActionIndices` dynamic index**: not explicitly called
|
|
out in tasks.md's acceptance criteria for 3.7, but necessary for
|
|
correctness — when `_colaLocal != null`, `skipToPrevious` is prepended
|
|
at index 0, so the compact-view highlighted action (play/pause) index
|
|
shifts to `1` (`colaActiva ? 1 : 0`) to keep pointing at play/pause
|
|
instead of skip-previous. The `_colaLocal == null` branch remains `[0]`,
|
|
unchanged.
|
|
4. **Phase 5.2 test placement** — see "Regression discipline" above: added
|
|
to a new file, not the protected `servicio_audio_reconnect_test.dart`,
|
|
to preserve that suite's exact pass count.
|
|
|
|
## Risks / follow-ups for verify phase
|
|
|
|
- Phase 3's handler wiring (the highest-risk part of this change) is
|
|
static-review-only by design — `PluriWaveAudioHandler` cannot be
|
|
instantiated in unit tests. Correctness there rests on (a) the pure
|
|
`cola_local.dart` functions being fully unit-tested and (b) careful
|
|
manual diff review of the wiring, both completed in this pass, but a
|
|
human/verify-phase re-read of `servicio_audio.dart`'s diff is still
|
|
warranted given the stakes.
|
|
- Phase 6.1 (`flutter analyze`, `flutter test --coverage`, `flutter
|
|
gen-l10n`) was explicitly out of scope for this apply pass (strict-TDD
|
|
mode: only target test files were run) and remains a manual follow-up
|
|
before merge.
|
|
- On-device / DHU manual QA for real car transport buttons and a genuine
|
|
`ProcessingState.completed` emission is out of scope here (session
|
|
precedent, per design's Testing Strategy table) and should be scheduled
|
|
separately.
|
|
|
|
## Skill compliance
|
|
|
|
- `literal-encoding`: scan run over all touched/new `.dart` files
|
|
(`cola_local.dart`, `navegacion_auto.dart`, `servicio_audio.dart`,
|
|
`cola_local_test.dart`, `navegacion_auto_test.dart`,
|
|
`controlador_reconexion_local_test.dart`) for `Ã.|â€|�` — 0 matches,
|
|
passed.
|
|
- `work-unit-commits`: working tree left UNSTAGED per instruction — the
|
|
orchestrator handles commit/push with a `[size:exception]` tag. No
|
|
`git commit` was run by this agent.
|