Promotes the android-auto-media capability spec to openspec/specs/ and moves both completed changes into openspec/changes/archive/.
22 KiB
Tasks: auto-media-art-quality
Strict TDD active for Dart layers. Behavioral task = RED (failing test) -> GREEN (minimal impl) -> REFACTOR (cleanup, still green). Native/XML/PNG/build.gradle tasks have no flutter test coverage (no Android build env) — marked [static-review-only]; flutter build/flutter run/flutter analyze MUST NOT be executed in this environment (they hang). "Parallel" tasks have no file overlap with concurrently-listed siblings; "Sequential" tasks depend on a prior task's output.
Affected files: lib/servicios/navegacion_auto.dart, android/app/src/main/res/drawable/station_art_aurora.png, android/app/src/main/res/drawable/station_art_cosmic.png, android/app/src/main/res/drawable/station_art_pulse.png, android/app/src/main/res/drawable/station_art_nova.png, android/app/src/main/res/drawable/default_station_art.png (deleted), test/servicios/navegacion_auto_test.dart.
Baseline (verified against live code, not trusted from spec/design prose):
lib/servicios/navegacion_auto.dart:12-17—_defaultArtUriconst, to be dropped.lib/servicios/navegacion_auto.dart:102-108—itemEmisora(), setsartUrionly, nodisplaySubtitle.lib/servicios/navegacion_auto.dart:110-113—_artUriPara(e): null/empty favicon check only, no URL-shape validation.lib/widgets/tarjeta_emisora.dart:361-370—_fallbackArtFor(seed):[aurora, cosmic, pulse, nova]order,seed.codeUnits.fold<int>(0,(a,b)=>a+b) % arts.length, assets atassets/images/station_art_<name>.png.lib/modelos/emisora.dart:60-78—Emisora.fromMap(SQLite favorites row) assignsfavicon: map['favicon'] as String?with no sanitization (unlikeEmisora.fromApi:47, which runs_nonEmpty). Confirms design.md's claim: malformed/whitespace favicon values from favorites rows reach the tree unfiltered today.android/app/src/main/res/drawable/default_station_art.pngexists (only PNG currently in that directory) — to be deleted.assets/images/station_art_{aurora,cosmic,pulse,nova}.pngexist — source images to copy intoandroid/app/src/main/res/drawable/.android/app/build.gradle.kts:34—applicationId = "es.freetimelab.pluriwave", matches the existingandroid.resource://es.freetimelab.pluriwave/drawable/...authority used by_defaultArtUriand the design's newstation_art_<name>URIs. No manifest/build.gradle change is needed — addingres/drawablePNGs requires no manifest declaration (unlike the parent change'sautomotive_app_desc.xmlmeta-data, which pointed at an XML resource).test/servicios/navegacion_auto_test.dart:45-63— existing test asserts fallback to_defaultArtUrion null/empty favicon; must be replaced (not just extended) since_defaultArtUriis retired.audio_service-0.18.18(lib/audio_service.dart:622):MediaItem.displaySubtitleisString?— nullable, safe to pass a nullablesubtituloCalidad(e)result directly.
Phase 1 — Favicon validity gate (faviconUsable)
1.1 [x] [RED] faviconUsable rejects malformed/non-http(s) favicons, accepts valid http(s) (Sequential — first task, defines the seam)
- Satisfies: Design Decision "Case B detection" (static validity gate); Spec "Playable Item Metadata" / Scenario "Station's logo URL is present but unreachable (Case B)" (malformed subset).
- File:
test/servicios/navegacion_auto_test.dart. - New
group('faviconUsable', ...). Table-driven cases, each assertingfaviconUsable(input) == expected:null→false''→false' '(whitespace only) →false'ftp://cdn.example.com/logo.png'(non-http(s) scheme) →false'cdn.example.com/logo.png'(no scheme, bare host) →false'http://'(scheme present, no authority) →false'/relative/path/logo.png'(relative, no scheme) →false'not a url at all $$$ ///'(unparseable/garbage) →false'http://cdn.example.com/logo.png'→true'https://cdn.example.com/logo.png'→true
- Run: fails (
faviconUsablenot implemented / not exported).
1.2 [x] [GREEN] Implement faviconUsable (Sequential — depends on 1.1)
- File:
lib/servicios/navegacion_auto.dart. - Top-level (or static)
bool faviconUsable(String? favicon): trim; if null/empty after trim →false.Uri.tryParse(trimmed); if parse fails →false. Requirescheme == 'http' || scheme == 'https'ANDuri.hasAuthority(non-empty host) →true, elsefalse. - Run: 1.1 passes.
Phase 2 — Rotation parity (indiceArtePara)
2.1 [x] [RED] indiceArtePara matches tarjeta_emisora.dart's _fallbackArtFor formula and order (Sequential — depends on Phase 1 file existing, independent logic)
- Satisfies: Design Decision "Fallback-art selection — port
_fallbackArtForverbatim"; Spec "Fallback art matches phone-UI per-station selection". - File:
test/servicios/navegacion_auto_test.dart. - New
group('indiceArtePara', ...). AssertindiceArtePara(seed)for a handful of sample uuids reproducesseed.codeUnits.fold<int>(0,(a,b)=>a+b) % 4computed inline in the test (do not hardcode indices without showing the formula), covering at least: an empty string (''→ index 0, sincefoldover no elements is 0), a short seed, and a real-looking uuid. - Assert the canonical order constant (or equivalent) used for mapping index→name is exactly
['aurora', 'cosmic', 'pulse', 'nova'], matchingtarjeta_emisora.dart:362-367. - Run: fails.
2.2 [x] [GREEN] Implement indiceArtePara + canonical art-name list (Sequential — depends on 2.1)
- File:
lib/servicios/navegacion_auto.dart. int indiceArtePara(String seed) => seed.codeUnits.fold<int>(0, (a, b) => a + b) % 4;— verbatim port, same formula astarjeta_emisora.dart:368.- Private
const _nombresArte = ['aurora', 'cosmic', 'pulse', 'nova']— same order astarjeta_emisora.dart:363-366. Add a code comment cross-linking totarjeta_emisora.dart's_fallbackArtForto mitigate reorder drift (per design.md's stated mitigation, since there is no structural enforcement of order — see Phase 6 for the added test-level guard). - Run: 2.1 passes.
Phase 3 — artUriPara integration (favicon gate + rotating drawable, retires _defaultArtUri)
3.1 [x] [RED] artUriPara returns the favicon when usable, a rotating station_art_<name> drawable URI otherwise (Sequential — depends on 1.2, 2.2)
- Satisfies: Design Decision 1 (validity gate) + Decision 2 (rotation) combined at the
artUriParaseam; Spec "Playable Item Metadata" (all 5 scenarios). - File:
test/servicios/navegacion_auto_test.dart. - New
group('artUriPara', ...):- Valid
https://favicon →artUriPara(e) == e.favicon. nullfavicon →artUriPara(e) == 'android.resource://es.freetimelab.pluriwave/drawable/station_art_<expected-name-for-uuid>'(compute expected name viaindiceArtePara(e.uuid)+ canonical order in the test, not a hardcoded guess).''favicon → same rotating-drawable behavior as null.- Malformed favicon (e.g.
'ftp://x/y.png','not a url') → same rotating-drawable behavior (Case B malformed subset). - Two different uuids that hash to different indices → assert their resolved
artUriParavalues differ (parity smoke check, not full enumeration — that's Phase 6). - Same uuid called twice → identical result (determinism).
- Valid
- Run: fails.
3.2 [x] [GREEN] Implement artUriPara; drop _defaultArtUri and the old _artUriPara (Sequential — depends on 3.1)
- File:
lib/servicios/navegacion_auto.dart. - Replace
_artUriPara(lines 110-113) withString artUriPara(Emisora e) => faviconUsable(e.favicon) ? e.favicon! : 'android.resource://es.freetimelab.pluriwave/drawable/station_art_${_nombresArte[indiceArtePara(e.uuid)]}';. - Delete
_defaultArtUriconst (lines 12-17) — dead now that rotation is total (design.md Decision 3: uuid always present, empty-uuid → index 0 → aurora, no 5th fallback reachable). - Update
itemEmisora()(line 106) call site from_artUriPara(e)toartUriPara(e). - Run: 3.1 passes.
3.3 [x] [GREEN] Replace the obsolete default-art test in itemEmisora group (Sequential — depends on 3.2)
- File:
test/servicios/navegacion_auto_test.dart. - Replace the test at lines 45-63 (
'cae al arte por defecto cuando el favicon es null o vacío', which asserts the now-deleted_defaultArtUriliteral) with an assertion that both null- and empty-favicon cases resolve to the correct rotatingstation_art_<name>URI perindiceArtePara, consistent with the Phase 3.1 cases. - Run: full
navegacion_auto_test.dartgreen.
Phase 4 — Quality subtitle (subtituloCalidad)
4.1 [x] [RED] subtituloCalidad matrix: both known, codec-only, bitrate-only, both unknown, bitrate<=0 (Sequential — independent of Phase 1-3, same file)
- Satisfies: Design Decision "
displaySubtitlequality format"; Spec "Station has known codec and bitrate" / "Station has unknown codec or bitrate". - File:
test/servicios/navegacion_auto_test.dart. - New
group('subtituloCalidad', ...), one case per design.md's table plus thebitrate <= 0edge (Radio Browser stores 0 for unknown):codec: 'mp3', bitrate: 128→'128 kbps · MP3'(codec upper-cased;·is U+00B7).codec: null, bitrate: 128→'128 kbps'.codec: 'mp3', bitrate: null→'MP3'.codec: 'mp3', bitrate: 0→'MP3'(bitrate ≤ 0 treated as unknown).codec: null, bitrate: null→null(not'', not the literal string'null').codec: null, bitrate: 0→null.codec: ' '(whitespace-only codec),bitrate: null→null(whitespace codec counts as unknown per design.md's "empty/whitespace codec count as unknown").codec: ' mp3 '(whitespace-padded codec),bitrate: 128→'128 kbps · MP3'(trimmed).
- Explicitly assert none of the outputs contain the substring
'null'(defends the "never render literal null" requirement directly, not just via exact-match). - Run: fails.
4.2 [x] [GREEN] Implement subtituloCalidad (Sequential — depends on 4.1)
- File:
lib/servicios/navegacion_auto.dart. String? subtituloCalidad(Emisora e): normalize codec viatrim().toUpperCase(), treat empty-after-trim as unknown; treatbitrate == null || bitrate! <= 0as unknown.- Both known →
'$bitrate kbps · $codecUpper'. Bitrate only →'$bitrate kbps'. Codec only →codecUpper. Both unknown →null(never'', never a string containing'null'). - Run: 4.1 passes.
Phase 5 — itemEmisora wiring (displaySubtitle)
5.1 [x] [RED] itemEmisora sets displaySubtitle from subtituloCalidad; omits it (leaves null) when both unknown (Sequential — depends on 3.2, 4.2)
- Satisfies: Spec "Browsable Media Tree" / Scenario "Station has known codec and bitrate" + "Station has unknown codec or bitrate", applied at the leaf-building integration point.
- File:
test/servicios/navegacion_auto_test.dart. - Extend the existing
group('ConstructorArbolAuto.itemEmisora', ...):Emisorawithcodec: 'mp3', bitrate: 128→item.displaySubtitle == '128 kbps · MP3'.Emisorawithcodec: null, bitrate: null→item.displaySubtitle == null.
- Run: fails (current
itemEmisoraat lines 102-108 never setsdisplaySubtitle).
5.2 [x] [GREEN] Wire displaySubtitle into itemEmisora (Sequential — depends on 5.1)
- File:
lib/servicios/navegacion_auto.dart. - Add
displaySubtitle: subtituloCalidad(e)to theMediaItem(...)construction initemEmisora(lines 102-108), alongside the existingartUri: Uri.parse(artUriPara(e)). - Run: 5.1 passes; full
itemEmisoragroup green.
5.3 [x] [REFACTOR] Cleanup pass on navegacion_auto.dart (Sequential — depends on 3.2, 4.2, 5.2 all green)
- Re-read top to bottom: doc-comment
faviconUsable/artUriPara/indiceArtePara/subtituloCalidadper the Design "Interfaces / Contracts" signatures; confirm no leftover reference to_defaultArtUrior the old_artUriParaanywhere in the file (grep the file for both identifiers — should return zero matches). - Full
navegacion_auto_test.dartsuite stays green.
Phase 6 — Downstream coverage & drift guards (favorites sanitization gap, phone/car order parity)
6.1 [x] [RED→GREEN, coverage-only] Test that faviconUsable/artUriPara catches the unsanitized Emisora.fromMap favicon gap (Sequential — depends on 3.2)
- Satisfies: Design Decision 1's explicit callout that
Emisora.fromMap(SQLite favorites,lib/modelos/emisora.dart:60-78) does not sanitizefavicon(no_nonEmpty, unlikeEmisora.fromApi:47) — confirms the validity gate is the correct place to catch this, rather than silently assuming it. - File:
test/servicios/navegacion_auto_test.dart. - Build an
EmisoraviaEmisora.fromMap({...})(not the_emisora()test helper) with a malformed/whitespace favicon value in the map (e.g.'favicon': ' ', or'favicon': 'not-a-url') thatfromMappasses through unsanitized. AssertartUriPara(builtEmisora)resolves to a rotatingstation_art_<name>URI, not the malformed string. - This is a regression-style test, not a design-doc gap-fix: it documents the current architecture's division of responsibility (sanitize-at-read-boundary vs sanitize-at-render-boundary) and pins it in place. If it fails,
fromMapstarted sanitizing (fine) or the gate regressed (not fine) — either way the test makes the boundary explicit instead of assumed. - Run: passes on first run once Phase 3 lands (this is a characterization test of already-implemented behavior, so RED/GREEN collapses to a single assertion pass — still write it before considering Phase 3 "done" in spirit, but there is no separate implementation step).
6.2 [x] [RED→GREEN, coverage-only] Parity guard: phone-UI asset order and Android-Auto drawable order stay identical (Sequential — depends on 2.2)
- Satisfies: Design.md's explicitly flagged risk — "reorder drift" between
tarjeta_emisora.dart's_fallbackArtForasset list andnavegacion_auto.dart's_nombresArtelist, called out as having "no structural enforcement," mitigated only by a code comment + this test. - File:
test/servicios/navegacion_auto_test.dart. - New
group('parity: phone/auto art order', ...). Since_fallbackArtFor's asset list is private totarjeta_emisora.dartand returnsassets/...paths (not directly importable/comparable in form), this test hardcodes the canonical order['aurora', 'cosmic', 'pulse', 'nova']once, with a comment explaining it must be kept in sync withtarjeta_emisora.dart:363-367's literal list, and assertsnavegacion_auto.dart's exposed art-name list/mapping matches it element-for-element, in order. - This does not eliminate drift risk (design.md accepts that — no structural enforcement chosen) but ensures any future reorder in
navegacion_auto.dartalone breaks a test immediately; a reorder intarjeta_emisora.dartalone still requires a human to update this test's hardcoded list (documented limitation, same as design.md's own acknowledgment). - Run: passes once Phase 2 lands.
Phase 7 — Android native drawables (retiring default_station_art.png) [static-review-only — no Android build env, flutter build/flutter run/flutter analyze MUST NOT be executed]
7.1 [x] Add 4 native drawable copies (Parallel with 7.2 — different files within the same directory, no overlapping content)
- Satisfies: Spec "Fallback art matches phone-UI per-station selection" + "Fallback art is on-brand, not the launcher icon"; Design "Native drawables + retire
default_station_art.png". - Files:
android/app/src/main/res/drawable/station_art_aurora.png,station_art_cosmic.png,station_art_pulse.png,station_art_nova.png(new binary assets). - Copy verbatim from
assets/images/station_art_{aurora,cosmic,pulse,nova}.png— same technique the parent change used fordefault_station_art.png(Phase 6.3 there). - Static review: filenames match exactly (case-sensitive) the names produced by
_nombresArte/indiceArteParainnavegacion_auto.dart(station_art_<name>, no extension in theandroid.resource://URI), matching the pattern already verified working fordefault_station_art.
7.2 [x] Delete default_station_art.png (Parallel with 7.1 — different file)
- Satisfies: Design Decision "Native drawables + retire
default_station_art.png" — rotation is total (uuid always present, empty-uuid → index 0 → aurora), so this drawable is unreachable dead weight and a byte-for-byte launcher-icon lookalike (parent change's WARNING #2). - File:
android/app/src/main/res/drawable/default_station_art.png(delete). - Static review: confirm no remaining reference to
default_station_artanywhere in the codebase after Phase 3.2 removes the Dart-side const (grepdefault_station_artacrosslib/andandroid/— should return zero matches after this task, other than this tasks.md/spec/design history).
7.3 [x] Verify applicationId/authority match for the new drawable URIs (Parallel with 7.1/7.2 — read-only verification, no file changes expected)
- Satisfies: precedent from parent change's WARNING notes (authority/applicationId must match for
android.resource://URIs to resolve). - Already verified during task planning:
android/app/build.gradle.kts:34setsapplicationId = "es.freetimelab.pluriwave", matching the authority segment (es.freetimelab.pluriwave) used in both the retired_defaultArtUriand the newstation_art_<name>URIs built inartUriPara(Phase 3.2). No manifest or build.gradle change is required — adding PNGs tores/drawableneeds no manifest declaration, unlike the parent change'sautomotive_app_desc.xmlmeta-data entry (ares/xmlresource that did require a<meta-data>reference). - Static review only: re-confirm this holds at apply time (
applicationIdunchanged) before closing Phase 7.
Phase 8 — Full regression sweep (Sequential — depends on all prior phases green)
8.1 [x] Run full flutter test suite (or the closest safe equivalent in this environment)
- Satisfies: Design "Testing Strategy" unit rows; Spec-level success criteria across all modified scenarios.
- Run the entire suite to catch cross-file regressions, not just
navegacion_auto_test.dart. - Expected deviation (execution constraint, same as parent change): if a full-suite run is not safely executable in this environment, run targeted files instead — at minimum
navegacion_auto_test.dartin full — and record which files were actually exercised. A genuine full-suite run is recommended before merge/CI, same caveat the parent change recorded.
8.2 [x] flutter analyze — DO NOT RUN in this environment
- DEVIATION (execution constraint, same as parent change):
flutter analyzehangs in this environment and MUST NOT be executed. Perform manual static review instead: confirmartUriPara,faviconUsable,indiceArtePara,subtituloCalidadare used (no unused-symbol risk), confirm no leftover references to_defaultArtUri/old_artUriPara, confirm all new code compiles implicitly by virtue of the modified file's tests passing in 8.1. A realflutter analyzepass is still recommended before merge/CI.
8.3 [x] flutter build/flutter run — DO NOT RUN in this environment
- DEVIATION (execution constraint, same as parent change): these commands hang in this environment and MUST NOT be executed. Phase 7's native drawable work is static-review-only for this reason; a real Android build (or DHU session) is required before shipping to confirm the 4 new
station_art_*drawables resolve correctly viaandroid.resource://(Design's remaining Open Question).
8.4 [x] Manual DHU verification note (optional, non-blocking, not coded)
- Satisfies: Design "Testing Strategy" manual row; Design "Open Questions" (confirm OS art loader accepts
android.resource://for the 4 new drawables). - Record in the commit/PR description that a Desktop Head Unit (or real car) session is recommended before shipping to visually confirm: (a) all 4 rotating arts render correctly per station, (b)
displaySubtitlequality text renders as expected on the browse row, (c) no staledefault_station_artreference lingers anywhere in the built APK's resources.
Review Workload Forecast
- Files touched:
lib/servicios/navegacion_auto.dart(~40-60 changed/added lines: drop_defaultArtUri(~6 lines removed), addfaviconUsable,indiceArtePara,_nombresArte,artUriPara,subtituloCalidad(~35-45 lines added), one-linedisplaySubtitlewiring initemEmisora),test/servicios/navegacion_auto_test.dart(~130-170 added/changed lines: 6 new test groups —faviconUsable,indiceArtePara,artUriPara,subtituloCalidad,itemEmisoraextension, parity guard — plus replacing the obsolete default-art test), 4 new binary PNGs underandroid/app/src/main/res/drawable/(station_art_aurora/cosmic/pulse/nova.png, copied from existingassets/images/, zero text-line cost), 1 binary deletion (default_station_art.png). - Estimated total changed/added lines (text only, excluding binary PNG bytes): roughly 170-230 lines.
- Chained PRs recommended: No — this is well inside the 400-line single-PR budget even with generous test-heavy estimates. No manifest/build.gradle changes are needed (verified:
applicationIdalready matches,res/drawablePNGs need no manifest declaration), which removes the one file class that made the parent change's Phase 6 riskier. - 400-line budget risk: Low. Total estimate (~170-230 text lines + 4 binary adds + 1 binary delete) sits comfortably under budget as a single commit/PR.
- Decision needed before apply: No — proceed as a single delivery. The only structural caveat carried over from the parent change is that Phase 7 (native drawables) remains static-review-only (no Android build env here), same constraint class as the parent change's Phase 6, but it does not change file-count or line-count risk enough to warrant chaining.
- Ownership/dependency note: Phase 1 (validity gate) and Phase 2 (rotation) are independent of each other and can proceed in any order — both are prerequisites for Phase 3 (
artUriParaintegration), which retires_defaultArtUri. Phase 4 (subtitle) is fully independent of Phases 1-3 (different concern, same file) and could be done in parallel by a second contributor without conflict risk beyond normal same-file merge care. Phase 5 depends on both Phase 3 and Phase 4 landing. Phase 6 is coverage-only and depends on Phase 3 (6.1) and Phase 2 (6.2) respectively but not on each other. Phase 7 has zero Dart dependency and may be done anytime in parallel with all Dart phases (mirrors the parent change's Phase 6 independence). Phase 8 is a hard sequential gate depending on everything above.