Move the change folder to changes/archive/2026-07-11-bt-device-identity with the verified artifact set (verdict: pass with warnings, 0 critical, 102/102 targeted tests) and create the bt-device-identity capability spec under openspec/specs/. Phase 7 on-device QA remains the pending human gate before release.
325 lines
31 KiB
Markdown
325 lines
31 KiB
Markdown
# Verification Report
|
|
|
|
**Change**: bt-device-identity
|
|
**Version**: N/A (spec has no version field)
|
|
**Mode**: Strict TDD (Dart layers, cross-referenced against real `flutter test` runs) / code-inspection only for Kotlin (no Android/Kotlin test harness exists in this repo - confirmed, `test/` tree is Dart-only)
|
|
**Commits verified**: `aef4e02` (Kotlin plumbing) + `b17c582` (Dart state/display/migration). Working tree clean of `lib/`/`android/` changes at verification time.
|
|
|
|
> **Scenario count correction**: the verification brief cited "24 scenarios." Independent counts from both the engram spec artifact (`sdd/bt-device-identity/spec`, obs #2304) and the file `openspec/changes/bt-device-identity/spec.md` (`grep -c "^##### Scenario:"`) agree: **21 scenarios across 7 requirement groups**. This report verifies against the confirmed 21.
|
|
|
|
---
|
|
|
|
## Completeness
|
|
|
|
| Metric | Value |
|
|
|--------|-------|
|
|
| Tasks total (tasks.md, grep-verified) | 56 |
|
|
| Tasks complete `[x]` (Phase 1-5) | 40 |
|
|
| Tasks N/A with inline justification (Phase 6, l10n) | 6 |
|
|
| Tasks pending - human sign-off gate (Phase 7, manual/on-device QA) | 10 |
|
|
| Tasks incomplete/unaccounted | 0 |
|
|
|
|
Independently re-counted via `grep -c` against the live `tasks.md` file - matches apply-progress self-reported totals exactly (40 checked, 16 unmarked = 6 N/A + 10 pending).
|
|
|
|
---
|
|
|
|
## Build & Tests Execution
|
|
|
|
**Build**: not run (per verification scope - `flutter build` explicitly excluded).
|
|
|
|
**Tests**: independently re-executed in this pass, not taken on the apply-progress report alone.
|
|
|
|
```text
|
|
flutter test test/estado/estado_ecualizador_test.dart test/servicios/servicio_ecualizador_test.dart \
|
|
test/pantallas/pantalla_ajustes_test.dart test/servicios/servicio_dispositivo_audio_test.dart \
|
|
test/servicios/servicio_dispositivo_audio_real_test.dart test/servicios/servicio_dispositivo_audio_toggle_test.dart
|
|
-> 00:11 +101: All tests passed!
|
|
```
|
|
|
|
Per-file breakdown (re-run individually to cross-check against apply-progress claimed counts):
|
|
|
|
| File | Actual | Claimed (apply-progress) | Match |
|
|
|------|--------|---------------------------|-------|
|
|
| servicio_dispositivo_audio_real_test.dart | 10/10 | 10/10 | Yes |
|
|
| servicio_dispositivo_audio_test.dart | 7/7 | 7/7 | Yes |
|
|
| servicio_dispositivo_audio_toggle_test.dart | 5/5 | 5/5 | Yes |
|
|
| estado_ecualizador_test.dart | 48/48 | 48/48 | Yes |
|
|
| servicio_ecualizador_test.dart | 20/20 | 20/20 | Yes |
|
|
| pantalla_ajustes_test.dart | 11/11 | 11/11 | Yes |
|
|
|
|
**flutter analyze**: 0 issues, whole project ("No issues found!").
|
|
|
|
**Coverage**: not run this pass (line-coverage percent was not part of the verification checklist; scenario-level mapping below is more precise for this purpose).
|
|
|
|
---
|
|
|
|
## Spec Compliance Matrix
|
|
|
|
| # | Requirement | Scenario | Test / Evidence | Result |
|
|
|---|-------------|----------|------------------|--------|
|
|
| 1 | BLUETOOTH_CONNECT at point-of-intent | permission requested on UI open (manual QA) | Deferred - tasks.md 7.2 | DEFERRED |
|
|
| 2 | BLUETOOTH_CONNECT at point-of-intent | permission not requested at app launch (manual QA) | Deferred - tasks.md 7.1 | DEFERRED |
|
|
| 3 | Placeholder MAC guarded | permission granted yields real MAC id (Dart-testable) | Covered implicitly by extensive real-MAC-id tests (3.1, D.4, 5.5a/b, etc.) - all pass | COMPLIANT |
|
|
| 4 | Placeholder MAC guarded | permission granted yields real MAC id (manual QA, Kotlin) | Deferred - tasks.md 7.3; code-inspected: guard present, MAC-present path unchanged | DEFERRED (code OK) |
|
|
| 5 | Placeholder MAC guarded | permission denied yields composite fallback, no crash (Dart-testable) | estado_ecualizador_test.dart 3.4 - PASS | COMPLIANT |
|
|
| 6 | Placeholder MAC guarded | multiple denied-permission devices do not collide (Dart-testable) | estado_ecualizador_test.dart 3.6 - PASS (see SUGGESTION-1 re: wording nuance) | COMPLIANT |
|
|
| 7 | Placeholder MAC guarded | placeholder never surfaces as a stored id (Dart-testable) | Structural: Kotlin guard (code-inspected) prevents emission + migration tests 5.1/5.9 purge legacy residue. No single dedicated processing test, but cross-layer enforced | COMPLIANT |
|
|
| 8 | Platform-name cache feeds display | platform name displays, no custom rename (Dart-testable) | pantalla_ajustes_test.dart 4.1 - PASS | COMPLIANT |
|
|
| 9 | Platform-name cache feeds display | custom rename overrides platform name (Dart-testable) | pantalla_ajustes_test.dart 4.4 - PASS (self-documented as triangulation, not fail-before RED; still a valid passing assertion) | COMPLIANT |
|
|
| 10 | Platform-name cache feeds display | no platform name yet falls back to raw id (Dart-testable) | pantalla_ajustes_test.dart 4.5 - PASS | COMPLIANT |
|
|
| 11 | Rename survives re-pair | rename persists after re-pair (Dart-testable via fake device stream) | No covering test found anywhere in the repo (searched both diffs and full test/ tree) | UNTESTED - CRITICAL |
|
|
| 12 | Rename survives re-pair | rename persists after re-pair (manual QA) | Deferred - tasks.md 7.5 (correctly scoped, unchecked) | DEFERRED |
|
|
| 13 | No duplicate entries on transient reports | repeated event for known id is a no-op (Dart-testable) | estado_ecualizador_test.dart 3.7 - PASS | COMPLIANT |
|
|
| 14 | No duplicate entries on transient reports | transient non-BT id does not corrupt BT entry (Dart-testable) | estado_ecualizador_test.dart 3.8 - PASS | COMPLIANT |
|
|
| 15 | One-time guarded migration | migration removes only exact placeholder entries (Dart-testable) | servicio_ecualizador_test.dart 5.1 - PASS | COMPLIANT |
|
|
| 16 | One-time guarded migration | matrix keys purge only placeholder segment (Dart-testable) | servicio_ecualizador_test.dart 5.2 - PASS | COMPLIANT |
|
|
| 17 | One-time guarded migration | near-miss keys preserved (Dart-testable) | servicio_ecualizador_test.dart 5.3 - PASS | COMPLIANT |
|
|
| 18 | One-time guarded migration | migration runs once (Dart-testable) | servicio_ecualizador_test.dart 5.4 - PASS (strengthened: re-seed-between-calls proves true flag short-circuit) | COMPLIANT |
|
|
| 19 | One-time guarded migration | no-op when BLUETOOTH_CONNECT never requested (Dart-testable) | servicio_ecualizador_test.dart 5.5 - PASS | COMPLIANT |
|
|
| 20 | l10n: rationale/notice strings in all locales | new keys present in every locale (arb parity, Dart-testable) | N/A - verified via diff grep: zero Text()/SnackBar/Dialog/l10n./AppLocalizations additions in either commit; zero .arb files touched | N/A (justified) |
|
|
| 21 | l10n: rationale/notice strings in all locales | rationale copy renders before OS dialog (manual QA) | N/A - same reason; tasks.md 7.9 correctly marked effectively N/A | N/A (justified) |
|
|
|
|
**Compliance summary**: 15/21 COMPLIANT, 4/21 correctly DEFERRED (manual/on-device QA, Phase 7 human gate), 2/21 correctly N/A (l10n, justified), 1/21 UNTESTED (CRITICAL).
|
|
|
|
---
|
|
|
|
## Correctness (Static Evidence) - Kotlin, code-inspection only
|
|
|
|
| Item | Status | Notes |
|
|
|------|--------|-------|
|
|
| AndroidManifest.xml declares BLUETOOTH_CONNECT | Implemented | L17, verified live file |
|
|
| requestBluetoothConnect() mirrors requestPostNotificationsPermission() | Implemented | Structural match: SDK-version gate, checkSelfPermission, requestPermissions, return true |
|
|
| Placeholder MAC guarded via named constant | Implemented | bluetoothMacPlaceholder = "02:00:00:00:00:00" (L41), used in guard condition, no bare string literal in the check itself |
|
|
| Composite fallback colon-sanitizes productName | Implemented | .replace(":", "-"), blank becomes "unknown" |
|
|
| Re-emit after grant (ADR-2) | Implemented | onRequestPermissionsResult branch for bluetoothConnectPermissionRequestCode calls getActiveAudioDevice() + audioDevicesSink success on PERMISSION_GRANTED; no-op on denial |
|
|
| Old dead "00:00:00:00:00:00" fallback removed | Confirmed removed | Zero occurrences anywhere in MainActivity.kt (grep-verified) - replaced, not extended, as required |
|
|
| Request codes distinct | Confirmed | 4821 (visualizer) / 4822 (notifications) / 4823 (bluetooth) - no collision |
|
|
|
|
---
|
|
|
|
## Coherence (Design) - ADR 1-6
|
|
|
|
| Decision | Followed? | Notes |
|
|
|----------|-----------|-------|
|
|
| ADR-1: request BLUETOOTH_CONNECT on UI open via new channel call | Yes, with a signed-off trigger-point deviation | Fires on toggle-turn-ON tap (_alternarMultiDevice), not a StatefulWidget on-build hook. Explicitly flagged in Batch 1 and re-confirmed in Batch 2; satisfies both spec MUSTs without an unrequested widget-type conversion |
|
|
| ADR-2: re-emit device list after grant | Yes | Verified in onRequestPermissionsResult (Kotlin) |
|
|
| ADR-3: placeholder guard + composite fallback bt_a2dp:name:$safeProductName | Yes | Verified in deviceToMap(); Dart's _prefijoPlaceholderCompuesto = 'bt_a2dp:name:' matches exactly, cross-layer consistent. Note: this ADR itself deviates from the spec requirement literal wording, see WARNING-1 |
|
|
| ADR-4: in-memory-only platform-name cache, not persisted | Yes | _nombresPlataforma is a plain Map, no SharedPreferences key added |
|
|
| ADR-5: migration location (ServicioEcualizador.cargar()) + guard flag eq_placeholder_purge_done_v1 | Yes | Verified; purges exact literal from all 3 maps as specified |
|
|
| ADR-6: transient duplicate-entry guard, combined with pre-existing esBase guard | Yes | Verified: !esBase && !esPlaceholderCompuesto && !_presetsDispositivo.containsKey(...) - correctly reads and extends the CURRENT code (post-eq-device-disconnect-revert), not a stale design-time snapshot |
|
|
|
|
---
|
|
|
|
## TDD Compliance (Strict TDD Mode active)
|
|
|
|
| Check | Result | Details |
|
|
|-------|--------|---------|
|
|
| TDD Evidence reported | Yes | Full TDD Cycle Evidence table present in apply-progress for all Phase 2-5 Dart tasks. Phase 1 (Kotlin) correctly has none, no test harness exists, explicitly scoped as code-inspection-only in both design and tasks |
|
|
| All tasks have tests | Yes (Dart scope) | 20 genuinely new tests across 5 files, matching every RED/GREEN task pair in tasks.md Phase 2-5 |
|
|
| RED confirmed (tests exist) | Yes | All claimed test files and test names verified present in the actual diff/live files |
|
|
| GREEN confirmed (tests pass) | Yes | 101/101 independently re-executed this pass, not taken on trust |
|
|
| Triangulation adequate | Yes, with 4 explicitly self-labeled approval/regression-lock tests (3.7, 3.8, 4.4, 4.5) | Correctly and transparently distinguished from true fail-before/pass-after RED in both tasks.md and apply-progress, no overclaiming found |
|
|
| Safety Net for modified files | Yes, with 2 numeric discrepancies found | See below |
|
|
|
|
**TDD Compliance**: 5/6 checks clean, 1 check (Safety Net) has minor reporting-accuracy issues, see WARNING-2.
|
|
|
|
**Safety-net cross-check** (pre-existing test counts, verified via `git show <parent>:<file> | grep -c "test("`):
|
|
|
|
| File | Apply-progress claim | Actual (git-verified) | Match |
|
|
|------|----------------------|------------------------|-------|
|
|
| servicio_dispositivo_audio_test.dart | 6/6 pre-existing | 6 | Yes |
|
|
| pantalla_ajustes_test.dart | 7/7 pre-existing | 7 | Yes |
|
|
| servicio_ecualizador_test.dart | 13/13 pre-existing | 13 | Yes |
|
|
| estado_ecualizador_test.dart | 40/40 pre-existing | 43 | No, off by 3 |
|
|
| servicio_dispositivo_audio_toggle_test.dart (ripple-fix file) | "7 pre-existing tests" (Test Summary prose) | 5 | No, off by 2 |
|
|
|
|
Both discrepancies are narrative/arithmetic only. Actual runtime results (48/48 and 5/5 respectively) were independently re-verified as correct in this pass. See WARNING-2.
|
|
|
|
---
|
|
|
|
## Test Layer Distribution
|
|
|
|
| Layer | New Tests | Files | Tool |
|
|
|-------|-----------|-------|------|
|
|
| Unit | 16 | servicio_dispositivo_audio_real_test.dart (3), servicio_dispositivo_audio_test.dart (1), estado_ecualizador_test.dart (5), servicio_ecualizador_test.dart (7) | flutter_test |
|
|
| Widget | 4 | pantalla_ajustes_test.dart (4) | flutter_test (testWidgets) |
|
|
| E2E | 0 | - | - |
|
|
| Total new | 20 | 5 files (+1 file with a 1-method ripple-fix, 0 new tests) | |
|
|
|
|
---
|
|
|
|
## Assertion Quality
|
|
|
|
Reviewed all 20 new tests plus the 1 ripple-fix. No tautologies, no ghost loops, no assertion-free tests found; every test exercises real production code (emitirDispositivo -> _onDispositivoCambiado, migrarClavesPlaceholder(), cargar(), real widget pumps, mocked MethodChannel) and asserts concrete, non-trivial values.
|
|
|
|
| File | Line(s) | Assertion | Issue | Severity |
|
|
|------|---------|-----------|-------|----------|
|
|
| pantalla_ajustes_test.dart | 4.6 (solicitarPermisoBluetoothCalls counter) | expect(fakeDispositivo.solicitarPermisoBluetoothCalls, equals(0/1/1)) | Mock-call-count pattern (skill's explicit watch-list item) | SUGGESTION (justified, see notes) |
|
|
|
|
**Assertion quality**: 0 CRITICAL, 0 WARNING, 1 SUGGESTION (noted for transparency, not blocking, see SUGGESTION-2 for rationale).
|
|
|
|
---
|
|
|
|
## Localization (Phase 6)
|
|
|
|
Confirmed N/A, independently verified (not just taken from apply-progress claim):
|
|
- `git diff aef4e02~1..b17c582 --stat -- lib/l10n/` returns empty (zero .arb files touched across both commits)
|
|
- `git diff aef4e02~1..b17c582 -- lib/` piped through a grep for Text(, Snackbar, SnackBar, Dialog, l10n., AppLocalizations additions returns zero matches
|
|
- tasks.md Phase 6 (6.1-6.6) correctly left as unchecked with inline "N/A" justification, not deleted, not silently marked done
|
|
|
|
---
|
|
|
|
## Phase 7 (Manual/On-Device QA)
|
|
|
|
All 10 tasks (7.1-7.10) correctly unchecked in tasks.md - human sign-off gate, explicitly out of scope for this code-correctness verify pass (no instrumented/emulator harness exists in this repo). Task 7.5 specifically covers the manual-QA half of the CRITICAL finding below (rename survives re-pair, on-device) and is appropriately still pending.
|
|
|
|
---
|
|
|
|
## Issues Found
|
|
|
|
### CRITICAL (1)
|
|
|
|
**CRITICAL-1 - Untested Dart-testable spec scenario: "rename persists after re-pair"**
|
|
|
|
Requirement group "Rename overlay survives re-pair under canonical id" (spec.md L91-106) has two scenarios: one Dart-testable, one manual/on-device QA. The manual scenario is correctly deferred (tasks.md 7.5). The Dart-testable scenario has no covering test anywhere in the repository. Confirmed by:
|
|
- Full diff review of test/estado/estado_ecualizador_test.dart and test/pantallas/pantalla_ajustes_test.dart (only 5 + 4 new tests added this batch, none combining rename-set + disconnect + reconnect-same-MAC)
|
|
- Full-tree search for renombrarDispositivo( call sites in tests - none is followed by a sequence of emitirDispositivo calls simulating disconnect/reconnect
|
|
- tasks.md Phase 3/4 itself never planned a task for this composed scenario (traces back to design's own Testing Strategy section, which also never named it) - this is a planning-stage gap inherited faithfully through apply, not a deviation introduced during implementation
|
|
- Mitigating context: the two constituent behaviors are each independently tested. Reconnect-does-not-duplicate-or-corrupt-an-entry is proven generically by the pre-existing D.4 regression test (eq-device-disconnect-revert), and custom-rename-wins-over-platform-name is proven by new test 4.4. Composing them carries low residual risk, but per verification rules a scenario is compliant only when an actual runtime test covers it; code-review confidence is not a substitute.
|
|
|
|
**Recommendation**: add one composed test to estado_ecualizador_test.dart (store a custom rename for a real-MAC id, emit disconnect, emit reconnect with the same id, assert the rename is still returned by nombreVisible/obtenerNombreDispositivo AND presetsDispositivo.length did not grow). Estimated effort: small, single test, no production-code changes expected (the underlying mechanism already looks correct by inspection). Route through sdd-apply for this one addition, then re-run sdd-verify before archiving.
|
|
|
|
### WARNING (2)
|
|
|
|
**WARNING-1 - Composite fallback id shape deviates from the spec requirement literal wording**
|
|
|
|
Spec requirement prose (spec.md L31) states the fallback id "MUST fall back to a composite bt_a2dp:<type>:<productName>-shaped id," with its own scenario example showing "bt_a2dp:8:AirPods Pro". The actual design (ADR-3) and implementation instead use "bt_a2dp:name:$safeProductName" (literal "name" segment, not the numeric type). This is a documented, deliberate design choice (ADR-3 rationale: preserves the single-leading-bt_a2dp:-segment invariant for eq_presets_matriz_v1's split-on-first-colon parsing), consistently implemented cross-layer (Kotlin's literal prefix matches Dart's _prefijoPlaceholderCompuesto exactly), and does not violate any scenario's actual testable MUST conditions (deterministic, not equal to the placeholder literal, no crash, all verified passing). Recommend a quick sign-off from the spec owner that this wording gap is accepted, since a literal re-read of the requirement text alone would suggest non-compliance.
|
|
|
|
**WARNING-2 - Minor arithmetic/count inaccuracies in apply-progress narrative (self-correcting pattern, no functional impact)**
|
|
|
|
Two small discrepancies found between apply-progress prose claims and git-verified reality (in addition to the "63 vs 56 total tasks" discrepancy apply-progress already self-corrected transparently):
|
|
1. TDD Cycle Evidence table states "40/40 pre-existing" for the estado_ecualizador_test.dart Phase-3 safety-net row; git-verified actual pre-existing count is 43.
|
|
2. Test Summary prose states "7 pre-existing tests in servicio_dispositivo_audio_toggle_test.dart"; the file actually contains 5.
|
|
|
|
Neither affects functional correctness - actual runtime results (48/48 and 5/5 respectively) were independently re-verified as accurate in this pass. Recommend correcting these two numbers in the artifact for future-reader accuracy, no code action needed.
|
|
|
|
### SUGGESTION (3)
|
|
|
|
**SUGGESTION-1 - "Does not collapse into one entry" wording is satisfied trivially, not by design**
|
|
|
|
Scenario "multiple denied-permission devices do not collide" (spec.md L52-57) says presetsDispositivo "MUST NOT collapse them into one entry." ADR-6's actual behavior is to create zero presetsDispositivo entries for ANY composite-placeholder id (not just to avoid collisions between them), confirmed by test 3.6 asserting presetsDispositivo.length == baseline (unchanged) after both devices connect. This technically satisfies the literal wording (zero entries is not "one collapsed entry") but is a broader suppression than a literal reading might imply (some readers might expect 2 distinct persisted entries). Recommend confirming this matches product intent, no code change implied, purely a documentation/interpretation checkpoint.
|
|
|
|
**SUGGESTION-2 - Mock-call-count assertion in test 4.6 (pantalla_ajustes_test.dart)**
|
|
|
|
expect(fakeDispositivo.solicitarPermisoBluetoothCalls, equals(...)) matches the Strict-TDD skill's explicit "mock call count" watch-list pattern. In this specific case it is justified: solicitarPermisoBluetooth() is a fire-and-forget side effect with no other externally observable consequence inside a widget test (the real effect is an OS permission dialog, unobservable in-process), so the call counter is the most direct available proxy for "did the trigger fire exactly once, and not on toggle-OFF." Flagged for transparency per protocol, not recommended for rework.
|
|
|
|
**SUGGESTION-3 - Pending Engram conflict markers on artifact observations**
|
|
|
|
Both sdd/bt-device-identity/tasks (obs #2314) and sdd/bt-device-identity/apply-progress (obs #2328) show a pending "contested by #obs-..." marker in mem_search results. Both resolved to their latest, highest-revision content when retrieved via mem_get_observation (Revisions: 3 and 2, respectively) and were used as authoritative for this report. Recommend resolving/judging these pending conflicts before sdd-archive to keep the artifact trail clean - housekeeping only, not a content-accuracy concern for this verification.
|
|
|
|
---
|
|
|
|
## Verdict
|
|
|
|
**FAIL** - blocked by exactly one CRITICAL finding (CRITICAL-1: untested Dart-testable spec scenario for rename-survives-re-pair). All other 20/21 scenarios are verified COMPLIANT or correctly DEFERRED/N/A; all 101 targeted tests independently re-executed and green; flutter analyze clean; all 6 design ADRs followed; l10n N/A correctly justified and independently confirmed; Phase 6/7 task states correctly reflect their true completion status. This is a narrow, well-scoped gap with a small, clearly-specified remediation, not a systemic implementation problem. Recommend one additional composed test via sdd-apply, then re-run sdd-verify before sdd-archive.
|
|
|
|
---
|
|
|
|
## Re-verify (targeted - 2026-07-11)
|
|
|
|
**Scope**: targeted re-verify, not a full re-verification. Confirms closure of CRITICAL-1 and WARNING-1 from the FAIL verdict above, and re-confirms WARNING-2 is still open/cosmetic. The 20/21 scenarios already verified COMPLIANT/DEFERRED/N/A in the original pass above are unchanged and were not individually re-audited in this pass; only the touched artifacts (one new test, one spec-wording edit) were inspected, plus a fresh full run of the one modified test file and a whole-project `flutter analyze`.
|
|
|
|
**What changed since the FAIL verdict**:
|
|
1. New composed regression test `3.10 rename persists after re-pair` added to `test/estado/estado_ecualizador_test.dart` (test-only, +58/-0).
|
|
2. `openspec/changes/bt-device-identity/spec.md` L31 and L50 rewritten to document the implemented composite shape `bt_a2dp:name:<sanitized productName>` (was `bt_a2dp:<type>:<productName>`, e.g. `bt_a2dp:8:AirPods Pro`).
|
|
|
|
**Commit note**: the task brief cited commit ab50df0 for the test addition. `git show ab50df0 --stat` resolves to an author/date/message/diff that is identical to the current HEAD commit 8cca7c3 (message: "test(devices): cover rename surviving a re-pair cycle end to end", 1 file, `test/estado/estado_ecualizador_test.dart`, 58 insertions, 0 deletions). Checked: `git merge-base --is-ancestor ab50df0 HEAD` returns false - ab50df0 is dangling, not reachable from current HEAD, while 8cca7c3 is. Consistent with a version-bump commit (158203f, "chore: bump version to 0.1.74+75") landing between b17c582 and this commit and the branch being rebuilt on top of it after the task brief was written. Content verified byte-identical under both references - this is hash drift, not a content discrepancy.
|
|
|
|
---
|
|
|
|
### CRITICAL-1 - CLOSED
|
|
|
|
Verified independently, not taken on the apply-progress/tasks narrative alone:
|
|
|
|
- **Test exists and passes**: `flutter test test/estado/estado_ecualizador_test.dart` (re-run this pass) -> 49/49 pass, including test 46 "3.10 rename persists after re-pair - composed regression (closes bt-device-identity CRITICAL-1)". Full output archived below.
|
|
- **Sequence matches the spec scenario exactly** (`openspec/changes/bt-device-identity/spec.md` L95-100, Requirement: Rename overlay survives re-pair under canonical id):
|
|
|
|
| Spec (L97-100) | Test (`estado_ecualizador_test.dart`, spans L1323-1379) |
|
|
|---|---|
|
|
| GIVEN a custom rename "My Headphones" is stored for bt_a2dp:AA:BB:CC:DD:EE:FF | L1328-1332 defines btDevice with that exact id; L1347 connects it via emitirDispositivo(btDevice); L1353 renombrarDispositivo(btDevice.id, 'My Headphones'); asserted at L1354-1357 |
|
|
| WHEN that device disconnects and reconnects, reporting the same MAC | L1361 emitirDispositivo(builtinSpeaker) (disconnect); L1365 emitirDispositivo(btDevice) again - same id (re-pair) |
|
|
| THEN the device row MUST still display "My Headphones" | L1373-1376 expect(eq.nombreVisible(btDevice.id, eq.nombrePlataforma(btDevice.id)), equals('My Headphones')) |
|
|
| AND no second/duplicate entry MUST appear in presetsDispositivo | L1369 expect(eq.presetsDispositivo.length, equals(cantidadTrasRenombrar)) |
|
|
|
|
The test additionally asserts the preset entry itself is untouched (L1371, presetsDispositivo[btDevice.id] unchanged), a stronger check than the spec literally requires - good practice, not scope creep.
|
|
- **Zero production-code changes**: `git show 8cca7c3 --stat` (content-identical to ab50df0 --stat) -> 1 file changed, test/estado/estado_ecualizador_test.dart, 58 insertions(+), 0 deletions(-). No lib/ or android/ diff. Confirms this was a genuine coverage gap, not a latent bug - the mechanism was already correct, composed from already-tested constituents D.4 (reconnect-dedup) and 4.4 (rename-priority).
|
|
- **Assertion-quality audit (Strict TDD Step 5f)**: read the full test body. 5 expect() calls, all comparing dynamically-captured runtime values (presetTrasConectar, cantidadTrasRenombrar) against fresh reads taken after the disconnect/re-pair cycle - no tautologies, no ghost loops, no assertion-free paths, no mock-call-count coupling. Every assertion exercises real production code (emitirDispositivo -> _onDispositivoCambiado, renombrarDispositivo, nombreVisible, nombrePlataforma). 0 CRITICAL, 0 WARNING.
|
|
- **TDD classification**: apply-progress transparently labels this "Not RED-first by design" - a coverage-closing composed test run against unmodified, already-correct code, passing on first execution. This is the same category the original report already accepted without penalty for tests 3.7, 3.8, 4.4, 4.5 (approval/regression-lock style, honestly disclosed rather than overclaimed as true fail-before RED). Consistent treatment applied here - no new TDD-process flag.
|
|
|
|
**Verdict on CRITICAL-1**: CLOSED. The Dart-testable scenario in requirement group "Rename overlay survives re-pair under canonical id" is now COMPLIANT. (Spec Compliance Matrix row 11 in the original report above is left unedited for audit-trail integrity - read it as superseded by this section, not as still-accurate.)
|
|
|
|
---
|
|
|
|
### WARNING-1 - CLOSED
|
|
|
|
Verified spec vs design vs implementation triangulation across all three layers:
|
|
|
|
| Layer | Evidence | Value |
|
|
|---|---|---|
|
|
| Spec | spec.md L31 (requirement prose) | "...id MUST fall back to a deterministic composite id keeping the single leading bt_a2dp: segment, with the product name colon-sanitized (implemented shape: bt_a2dp:name:<sanitized productName>, per design ADR-3 ...)" |
|
|
| Spec | spec.md L50 (scenario) | "THEN deviceId MUST be a deterministic composite id (implemented shape: bt_a2dp:name:AirPods Pro) ..." |
|
|
| Kotlin | MainActivity.kt L777-784 (guard + fallback block), literal at L783 | "bt_a2dp:name:$safeProductName" |
|
|
| Dart | estado_ecualizador.dart L33 | static const _prefijoPlaceholderCompuesto = 'bt_a2dp:name:'; |
|
|
|
|
All four now agree on the literal "name" marker segment. The spec no longer describes a shape (bt_a2dp:<type>:<productName>, e.g. bt_a2dp:8:AirPods Pro) that the implementation never produced. This was a documentation-only fix - no code changed: the same commit-stat evidence above shows zero lib/ or android/ diff, and openspec/changes/bt-device-identity/ is entirely untracked in git (status shows it as untracked), consistent with this being a working-tree file edit rather than a commit.
|
|
|
|
**Verdict on WARNING-1**: CLOSED.
|
|
|
|
---
|
|
|
|
### WARNING-2 - still open, accepted as cosmetic (no action taken, per task brief)
|
|
|
|
Unchanged from the original report: the two arithmetic discrepancies (40 vs. git-verified 43 pre-existing estado_ecualizador_test.dart tests; "7 pre-existing" vs. actual 5 in servicio_dispositivo_audio_toggle_test.dart) live only in the apply-progress engram narrative, not in the spec, code, or tests. Actual runtime results were independently re-verified as correct both in the original pass and again in this pass (49/49, up from 48/48 - exactly +1 for the one new test, arithmetically consistent with the claimed 48-pre-existing baseline for task 3.10 specifically, so no new discrepancy was introduced by this batch). Zero functional impact. Left genuinely open per the task brief ("no action taken") rather than silently marked closed; see final verdict rationale below for why this keeps the verdict at PASS WITH WARNINGS rather than a bare PASS.
|
|
|
|
---
|
|
|
|
### Independently re-executed evidence (this pass)
|
|
|
|
```text
|
|
$ flutter test test/estado/estado_ecualizador_test.dart
|
|
...
|
|
00:01 +41: platform-name cache (bt-device-identity Phase 3) 3.1 platform name is cached from a device-change event
|
|
00:01 +42: platform-name cache (bt-device-identity Phase 3) 3.4 composite-placeholder sentinel does not create device-list entry
|
|
00:01 +43: platform-name cache (bt-device-identity Phase 3) 3.6 multiple denied-permission devices do not collide
|
|
00:01 +44: platform-name cache (bt-device-identity Phase 3) 3.7 repeated event for known id is a no-op on preset creation (regression-lock)
|
|
00:01 +45: platform-name cache (bt-device-identity Phase 3) 3.8 transient non-BT id during pairing handshake does not corrupt the BT entry (regression-lock)
|
|
00:01 +46: platform-name cache (bt-device-identity Phase 3) 3.10 rename persists after re-pair - composed regression (closes bt-device-identity CRITICAL-1)
|
|
00:01 +47: FakeServicioEcualizador - nombresDispositivos preservation (WARNING-3) guardarPrincipal preserves nombresDispositivos
|
|
00:01 +48: FakeServicioEcualizador - nombresDispositivos preservation (WARNING-3) guardarActivo preserves nombresDispositivos
|
|
00:02 +49: All tests passed!
|
|
```
|
|
|
|
```text
|
|
$ flutter analyze
|
|
Analyzing pluriwave...
|
|
No issues found! (ran in 5.4s)
|
|
```
|
|
|
|
flutter build was NOT run, per task constraints (consistent with the original pass's scope exclusion).
|
|
|
|
---
|
|
|
|
### Housekeeping note (SUGGESTION-3, still open)
|
|
|
|
The pending Engram conflict marker flagged in SUGGESTION-3 of the original report (on sdd/bt-device-identity/tasks obs #2314 and sdd/bt-device-identity/apply-progress obs #2328) is still present, and this verify-report itself (obs #2329) now also shows a pending "contested by" marker as of this session's mem_search. All three resolved to their latest/highest-revision content when fetched via mem_get_observation and were used as authoritative for this re-verify, same as before. Still recommended: resolve/judge these pending conflicts before sdd-archive - housekeeping only, not a content-accuracy concern.
|
|
|
|
---
|
|
|
|
## Updated Verdict
|
|
|
|
**PASS WITH WARNINGS**
|
|
|
|
CRITICAL-1 is closed with real, independently-executed, assertion-quality-audited test evidence composing the exact spec scenario (spec.md L95-100), against zero production-code changes (confirming a coverage gap, not a functional bug). WARNING-1 is closed - spec, design (ADR-3), Kotlin, and Dart now agree byte-for-byte on the bt_a2dp:name:<sanitized productName> composite shape. WARNING-2 remains open by the task's own account (cosmetic arithmetic drift in an internal apply-progress narrative, zero functional/spec/code impact, independently reconfirmed accurate at the runtime-result level in both verify passes) - kept open rather than silently closed, since no correction was actually made to the artifact. Zero CRITICAL findings remain. All 3 original SUGGESTIONs remain open/non-blocking and unchanged; one housekeeping note (pending Engram conflict markers, now also touching this report) is carried forward.
|
|
|
|
**Recommendation**: proceed to sdd-archive. Phase 7 (manual/on-device QA, tasks 7.1-7.10 in tasks.md) remains genuinely pending - it always has been, is a human-owned gate with no CI/emulator harness in this repo, and was correctly treated as DEFERRED (not blocking) in the original pass. It stays orthogonal to this code-correctness verdict; the maintainer should track it separately (e.g., as an archive follow-up or explicit sign-off note) rather than treat it as a re-verify blocker.
|