FreeTLab
8f7ca8059b
_onDispositivoCambiado() bootstrapped a device-level preset entry for every never-seen device id, including the built-in speaker. That persistent level-3 entry masked later global-preset edits (level 3 beats level 4 on every resolution), so disconnecting a BT device or cold-starting without one could leave the EQ stuck on an outdated copy instead of the current global preset. The base speaker is now excluded from the first-seen bootstrap: disconnect and cold start always resolve through the live hierarchy. BT/wired/USB devices keep their bootstrap behavior unchanged.
12 KiB
Verification Report
Change: eq-device-disconnect-revert Version: N/A (delta spec, no prior openspec/specs/multi-device-eq/spec.md) Mode: Strict TDD
Completeness
| Metric | Value |
|---|---|
| Tasks total | 16 |
| Tasks complete | 15 |
| Tasks incomplete | 1 (5.1, manual on-device sanity, intentionally left open as a non-blocking human gate) |
Build and Tests Execution
Build: not run (explicitly excluded from this verification per instruction; flutter build must never be run in this phase)
Tests: 43 passed / 0 failed (target file); 68 passed / 0 failed (target file plus 3 related regression files)
flutter test test/estado/estado_ecualizador_test.dart --reporter expanded
-> 43/43 passed, including D.1-D.5 (new), 5.5a/5.5b, 5.7a/5.8b, 2.1-2.4, rename API, guardarPresetDispositivo groups
flutter test test/estado/estado_ecualizador_test.dart test/servicios/servicio_audio_eq_reapply_test.dart test/servicios/servicio_ecualizador_test.dart test/pantallas/pantalla_ajustes_test.dart --reporter expanded
-> 68/68 passed
flutter test --reporter compact (attempted full suite)
-> TIMED OUT / hung at ~300s. Confirmed known unrelated flake: runaway log spam originating in
test/estado/estado_alarmas_ejecuciones_test.dart and test/estado/estado_alarmas_snooze_test.dart
(repeating log line once per simulated minute). Process terminated cleanly by tool timeout; verified
no orphaned dart.exe processes afterward. Fell back to explicit targeted runs per instructions.
Mode used: TARGETED FALLBACK, not full suite.
Coverage: Diff-region (the 2 actual changed executable lines): 100 percent. Whole-file baseline (this test file alone): 59.3 percent (118/199 lines), pre-existing, not moved backward by this change. See Changed File Coverage below.
Spec Compliance Matrix
| Requirement | Scenario | Test | Result |
|---|---|---|---|
| Device-Event Resolve-and-Apply | Connect applies the active device preset | estado_ecualizador_test.dart, test 5.3b (pre-existing, unmodified) | COMPLIANT |
| Device-Event Resolve-and-Apply | Disconnect resolves and applies the base preset | estado_ecualizador_test.dart, test D.1 | COMPLIANT |
| First-Seen Bootstrap Must Not Hijack Base-Device Resolution | Disconnect to a never-seen builtin_speaker resolves from the hierarchy | estado_ecualizador_test.dart, test D.2 | PARTIAL, see WARNING-1 |
| Cold Start Applies Base Resolution, Never a Stale Persisted Preset | Cold start with no device connected applies the base preset | estado_ecualizador_test.dart, test D.3 | COMPLIANT |
| Connect-Disconnect-Reconnect Cycle Lands on the Correct Preset at Each Step | Full cycle honors station/matrix overrides at each step | estado_ecualizador_test.dart, test D.4 | COMPLIANT |
| Toggle-Off Behavior Is Byte-for-Byte Unchanged | Toggle off ignores device events entirely | estado_ecualizador_test.dart, tests 5.3c and 5.7a (pre-existing, unmodified) | COMPLIANT |
| (bonus, design Testing Strategy, not a distinct spec scenario) | Station override still wins over global on disconnect-to-base | estado_ecualizador_test.dart, test D.5 | COMPLIANT (supplementary coverage) |
Compliance summary: 6/6 formal spec scenarios compliant (5 fully compliant, 1 PARTIAL on literal scenario wording only; functional intent met and exceeded) plus 1 bonus scenario compliant.
Note: the verification task brief referenced "6 requirements / 7 scenarios". The actual persisted spec artifact (engram sdd/eq-device-disconnect-revert/spec #2306, byte-identical to openspec/changes/eq-device-disconnect-revert/specs/multi-device-eq/spec.md) contains 5 "Requirement:" headings and 6 "Scenario:" headings (Requirement 1 has 2 scenarios; Requirements 2-5 have 1 each). Counting design-driven bonus test D.5 as a 7th test case reconciles the "7" figure; there is no 6th formal requirement in the artifact.
Correctness (Static Evidence)
| Requirement | Status | Notes |
|---|---|---|
| Device-Event Resolve-and-Apply | Implemented | Single handler _onDispositivoCambiado, unconditional resolve (_resolverPresetActivo) plus apply (audio.aplicarPreset) at L228-231, no event-type branch |
| First-Seen Bootstrap Must Not Hijack Base-Device Resolution | Implemented, stronger than literal scenario text | esBase guard at L220-221 fully excludes altavozInterno from bootstrap; no entry is ever persisted for builtin_speaker, not even a correct-value one |
| Cold Start Applies Base Resolution | Implemented | cargarPersistido calls _sembrarDispositivoActual which calls _onDispositivoCambiado with the builtin_speaker default; confirmed via source read L141-196 |
| Connect-Disconnect-Reconnect Cycle | Implemented | No state mutation specific to disconnect beyond the guarded bootstrap; hierarchy is a pure function of station and device |
| Toggle-Off Behavior Byte-for-Byte Unchanged | Implemented | L211 early return untouched; guard changes are unreachable when eqMultiDeviceEnabled is false |
Coherence (Design)
| Decision | Followed | Notes |
|---|---|---|
| 1. Single handler, unconditional resolve+apply, no connect/disconnect branch on event type | Yes | Confirmed via git diff and full source read; L213 assignment and L228-231 resolve+apply untouched |
| 2. Bootstrap guard scoped to altavozInterno only, not all BT/USB/wired | Yes | esBase = dispositivo.tipo == TipoDispositivo.altavozInterno; wired/BT/USB still bootstrap on first-seen (confirmed via passing 5.5a, 2.2, and D.4 BT first-connect assertion) |
| 3. Deterministic re-resolution, not snapshot/restore | Yes | No snapshot storage added; _resolverPresetActivo remains the sole resolver |
| 4. Reuse aplicarPreset choke point | Yes | L229-230 unchanged |
| 5. Cold start trusts existing seed path, no new code | Yes | cargarPersistido to _sembrarDispositivoActual to _onDispositivoCambiado, verified unchanged |
| 6. Toggle-off invariance, L211 early return untouched | Yes | Line unchanged; 5.3c/5.7a/5.8b still pass |
TDD Compliance
| Check | Result | Details |
|---|---|---|
| TDD Evidence reported | Yes | apply-progress #2323 includes a TDD Cycle Evidence table (3-column RED/GREEN/REFACTOR plus prose, not the full 5-column template, see SUGGESTION-2) |
| All tasks have tests | Yes | 5/5 Phase-D tasks (D.1-D.5) have one dedicated test case each |
| RED confirmed, tests exist | Yes | All 5 test cases verified present via direct file read of test/estado/estado_ecualizador_test.dart |
| GREEN confirmed, tests pass | Yes | 5/5 pass on independent re-execution this session (43/43 in file; 68/68 in fallback regression run) |
| Triangulation adequate | Yes | Each Phase-D scenario has exactly one purpose-built, non-overlapping test; no scenario needed multiple variants |
| Safety Net for modified files | Yes | lib/estado/estado_ecualizador.dart (modified, not new); 43/43 full-file regression green, independently re-verified this session |
TDD Compliance: 6/6 checks passed
Test Layer Distribution
| Layer | Tests | Files | Tools |
|---|---|---|---|
| Unit | 5 (new, Phase D) | 1 (estado_ecualizador_test.dart) | package:test |
| Integration/Widget | 0 new | pre-existing pantalla_ajustes_test.dart used only as unrelated regression check | flutter_test widget testing |
| E2E | 0 | 0 | not installed |
| Total (new to this change) | 5 | 1 |
Changed File Coverage
| File | Line pct whole file | Diff-region coverage | Uncovered lines in diff region | Rating |
|---|---|---|---|---|
| lib/estado/estado_ecualizador.dart | 59.3 pct (118/199, baseline; file has many unrelated pre-existing methods not exercised by this single test file) | 100 pct (L220 esBase hit 2x; L221 guarded if hit 3x; L222-224 bootstrap body hit 3x via 5.5a/2.2/D.4; L228-231 resolve+apply hit continuously) | none | Excellent (diff region) |
| test/estado/estado_ecualizador_test.dart | N/A, test file, not instrumented | - | - | - |
Source: coverage/lcov.info generated this session via flutter test --coverage test/estado/estado_ecualizador_test.dart (gitignored, not committed, not left in working tree changes).
Assertion Quality
All assertions verify real behavior. No tautologies, ghost loops, assertion-free tests, or CSS/implementation-detail-only checks found across D.1-D.5. Every test drives real production code (_onDispositivoCambiado, _resolverPresetActivo, cargarPersistido) and asserts on production-derived values (presetActual, presetsDispositivo, FakeServicioAudio.presetsAplicados), consistent with the spec instruction to assert presetsAplicados (proves push, not state-only mutation) rather than state alone.
Manually traced D.2 pre-fix versus post-fix behavior against the actual presetEfectivo hierarchy to independently confirm it is a genuine, non-tautological regression test (would have resolved presetActual == rock, stale, pre-fix versus pop, fresh, post-fix on the second builtin_speaker emit). This matches the claim already documented in apply-progress that D.2 is the only test among D.1-D.5 that targets the actual bootstrap-collision bug.
Assertion quality: All assertions verify real behavior.
Quality Metrics
Linter/Analyzer: No errors. flutter analyze reported zero issues (ran this session, full project, 63.6s) Type Checker: N/A separate tool; covered by flutter analyze (Dart static analysis)
Issues Found
CRITICAL: None
WARNING:
- Spec scenario text versus implementation divergence, Requirement 2 "First-Seen Bootstrap Must Not Hijack Base-Device Resolution". The spec scenario literally states that the presetsDispositivo entry for builtin_speaker is bootstrapped to rock (global fallback), implying the key gets created with the correct value. The actual implementation, matching Design Decision 2 and its explicit reasoning ("Rejected: keep and tolerate, leaves persisted mask bug"), NEVER creates that entry at all. The esBase guard unconditionally skips the whole bootstrap block, so the presetsDispositivo containsKey check for builtin_speaker stays false forever, proven by test D.2. This is objectively safer than the literal scenario (D.2 proves it prevents the L3-masks-L4 bug that a bootstrap-with-correct-value approach would not prevent) and is fully justified by the design document rationale; it is not a functional defect. But it means a reader following the spec literally would expect that containsKey check to be true, which is false: a real drift between spec-as-written and spec-as-implemented. Recommendation: before archive, update the spec scenario THEN clause to state that the entry is never bootstrapped and always resolves live through the hierarchy, so the spec matches the safer, already-implemented, already-tested behavior.
SUGGESTION:
- Reconcile spec text per WARNING-1 recommendation before archive (listed there to avoid double-counting).
- The apply-progress TDD Cycle Evidence table uses a 3-column format (RED/GREEN/REFACTOR plus prose) instead of the full 5-column strict-TDD template (RED/GREEN/TRIANGULATE/SAFETY NET/REFACTOR). Substance is all present and was cross-verified independently in this session, but a future sdd-apply run should use the standard table shape for easier automated auditing.
- The apply-progress "Issues Found" section already flags, correctly, as out-of-scope for this change, that cambiarPresetPrincipal immediate re-apply bypasses _resolverPresetActivo and applies the raw global value directly rather than through the hierarchy. Carrying this forward as a tracked tech-debt item; no action needed in this change, but worth a follow-up ticket since it could theoretically let a station/matrix override be transiently bypassed for one apply call when the global preset changes.
Verdict
PASS WITH WARNINGS
0 CRITICAL, 1 WARNING (spec-text/implementation wording drift on Requirement 2, functionally sound and safer than written; recommend a spec-text correction before archive), 3 SUGGESTION. All 43 tests in the target file pass (including all 5 new Phase-D tests and every named consistency lock: 5.5b, 5.7a, 5.8b, 2.2), flutter analyze is clean, the code change matches the design Bootstrap Guard block exactly (verified via git diff and direct source read), and 15/16 tasks are complete with task 5.1 intentionally left as the non-blocking manual on-device gate.