diff --git a/docs/superpowers/plans/2026-05-14-phase-b-dogfood-plan.md b/docs/superpowers/plans/2026-05-14-phase-b-dogfood-plan.md index 72b3f49..825bcaf 100644 --- a/docs/superpowers/plans/2026-05-14-phase-b-dogfood-plan.md +++ b/docs/superpowers/plans/2026-05-14-phase-b-dogfood-plan.md @@ -9,7 +9,7 @@ description: "Plan for Phase B of the post-rebrand dogfood pass — verify the 2 **Started:** 2026/05/14 **Owner:** Wren (with Jake oversight) -**Status:** In progress. B.1 done. B.2–B.15 pending. +**Status:** Complete 2026/05/14. B.1–B.15 audited. Nine surgical fixes shipped (B.1, B.2, B.3, B.4, B.5, B.6, B.7, B.8, B.9, B.10). Five documented passes (B.11, B.12, B.13, B.14, B.15) with verdict reasoning recorded below. --- @@ -45,20 +45,20 @@ For each B.x: | # | Surface | Commit(s) | Status | Outcome | |---|---|---|---|---| | **B.1** | Cancellable Whisper inference + bounded drain + lock-over-await | `5725836` | **Done** (`6c212a0`) | 8 existing unit tests cover the atomiser-targetable surface. Race-B end-to-end test acknowledged as hard-to-fixture in the SAFETY comment and left as-is. One real residual fixed: misleading comment on `start_live_transcription_session` claiming lifecycle is "Released explicitly before the RunningLiveSession is installed" — the code does the safer opposite (install-while-locked, drop after). Future reader would have trusted the comment and "fixed" the code into a regression. | -| **B.2** | Hotkey supervisor rearchitecture | `1068ad9` (Race-1, Race-2, TOCTOU) | Pending | — | -| **B.3** | Atomic model download + manifest | `9f67ab2` (Rev-1, Rev-5) | Pending | — | -| **B.4** | Soft-delete + trash + restore | `15b74db`, `87e6248`, `50d0715`, `99f4ecd` (Rev-2, Rev-3) | Pending | — | -| **B.5** | `write_text_file_cmd` path scoping + `transcribe_file` extension allowlist + size cap | `a2b47db`, `a48653c`, `b3da58c` (Trust-1) + `9653e25` (Trust-5) + `ed449cc` (Trust-2) | Pending | — | -| **B.6** | Main-window guards on clipboard / extract-tags / file-write | `12b413d`, `f7af7b0` (Trust-4), `7aee534` (Trust-3, Trust-6) | Pending | — | -| **B.7** | LlmEngine critical-section narrowing + drop-old-model-first | `cde985d` (Race-3, Lifecycle-1) | Pending | — | -| **B.8** | Span propagation across live + model-load spawns; `lumotia.log` writer + EnvFilter coverage; drop `lumotia_live` literal target | `65abfa2` (Obs-3), `8becb1a` (Obs-4, Obs-5), `d1391b3` (Obs-1, Obs-2) | Pending | — | -| **B.9** | JSON-envelope LLM extractor (GBNF removal) | `1d71e8e` | Pending | — | -| **B.10** | `focusTimer` rehydrate `startTick` invariant | `5ba761a` (Race-10) | Pending | — | -| **B.11** | `FirstRunPage` unlisten on all exits | `6aa6a43` (Race-9) | Pending | — | -| **B.12** | `Transcriber::transcribe_sync_with_abort` required | `e0e9a6e` (Lifecycle-2) | Pending | — | -| **B.13** | `drain_inference` deadline from `task.duration_secs` | `07f6755` (Time-bomb-1) | Pending | — | -| **B.14** | Surface capture-thread drops + bypass validation requeue cap | `094b533` | Pending | — | -| **B.15** | `test_llm_model` respects caller GPU preference | `afbd33d` (Race-8) | Pending | — | +| **B.2** | Hotkey supervisor rearchitecture | `1068ad9` (Race-1, Race-2, TOCTOU) | **Done** (`643985d`) | 6 unit + 2 integration tests already cover Race-1 / Race-2; TOCTOU honoured by author's `// TODO(test):` SAFETY annotation. Real residual fixed: `SHUTDOWN_TIMEOUT` const doc + test name `shutdown_force_aborts_stuck_tasks_after_timeout` both claim "force-abort" semantics, but `timeout(d, handle).await` consumes the JoinHandle by value — drop detaches, doesn't abort. Reworded doc + renamed test to match the actual detach contract. | +| **B.3** | Atomic model download + manifest | `9f67ab2` (Rev-1, Rev-5) | **Done** (`31e3f5a`) | Transcription side fully covered (resume, restart-on-200, SHA mismatch cleanup, 5xx rejection, Rev-1 preserve, Rev-5 manifest atomicity). One real residual in `crates/llm/src/model_manager.rs`: `ResumeUnsupported` returned without unlinking `.part`, so a transient server-downgrade leaves the download wedged forever. Fix unlinks `.part` before returning so the next retry starts fresh; new test `resume_unsupported_unlinks_part_so_retry_starts_fresh`. | +| **B.4** | Soft-delete + trash + restore | `15b74db`, `87e6248`, `50d0715`, `99f4ecd` (Rev-2, Rev-3) | **Done** (`20ef6c4`) | 4 backend tests cover soft-delete, audio cleanup, list-excludes/restore, purge. Stale TODO(test): vitest-not-installed comments on the Svelte UI components are now obsolete (vitest landed in Phase A.5). One real residual fixed: `purge_deleted_transcripts` was a SELECT-then-DELETE-WHERE-id-IN pair; a `restore_transcript` between the two statements would let the DELETE hard-delete the now-LIVE row + remove its audio, bypassing Rev-2's safety contract. Refactored to single `DELETE … RETURNING audio_path` for atomicity; new test `purge_audio_cleanup_only_fires_for_hard_deleted_rows`. | +| **B.5** | `write_text_file_cmd` path scoping + `transcribe_file` extension allowlist + size cap | `a2b47db`, `a48653c`, `b3da58c` (Trust-1) + `9653e25` (Trust-5) + `ed449cc` (Trust-2) | **Done** (`d8fa4ff`) | Trust-1 had 6 path-scope tests, Trust-5 had 7 extension/size tests, Trust-2 had 7 tests including symlink-pointing-out coverage. Real residual in Trust-1: asymmetric symlink handling vs Trust-2. `fs.rs` canonicalised only the parent of the target (file usually didn't exist yet), so a symlink AT the target path bypassed the containment check — `tokio::fs::write` then follows the symlink and writes outside the allowlist. Trust-2 already used full-path canonicalize on existing paths. Fix mirrors Trust-2's discipline: canonicalize full path if exists, fall back to parent canonicalize on NotFound. Two new symlink regression tests (in/out). | +| **B.6** | Main-window guards on clipboard / extract-tags / file-write | `12b413d`, `f7af7b0` (Trust-4), `7aee534` (Trust-3, Trust-6) | **Done** (`7f0e1b0`) | Strong coverage on size caps, terminal classification, paste-cap == clipboard-cap invariant. Real residual: the `12b413d` commit message claimed the `CLIPBOARD_ALLOWED_WINDOWS` / `PASTE_REPLACING_ALLOWED_WINDOWS` Rust consts mirror `secondary-windows.json`'s `windows` array, but no test pinned the invariant. A future drift between the Rust allowlists and the capability JSON silently disagrees the IPC trust boundary with the permission grant. Promoted both consts to `pub(crate)` and added `commands::security::tests_capability_mirror::allowlists_match_capability_jsons` which reads + parses the capability JSONs at test-time and asserts every Rust allowlist label is declared. | +| **B.7** | LlmEngine critical-section narrowing + drop-old-model-first | `cde985d` (Race-3, Lifecycle-1) | **Done** (`f252c1b`) | Two existing tests pin Race-3 (probes don't block on slow load) + Race-3/4 TOCTOU (parallel load refused with `AlreadyLoading`). Real residual: `unload()` did not consult the `loading` flag. Mid-load, `model` + `loaded` are already None (step 3 cleared them); concurrent unload no-op-clears, returns Ok, then step 5 installs the new state — caller saw unload-success but engine ends up loaded. Same flag now guards both directions. New test `unload_during_load_is_refused`; `EngineError::AlreadyLoading` message generalised to cover both directions. | +| **B.8** | Span propagation across live + model-load spawns; `lumotia.log` writer + EnvFilter coverage; drop `lumotia_live` literal target | `65abfa2` (Obs-3), `8becb1a` (Obs-4, Obs-5), `d1391b3` (Obs-1, Obs-2) | **Done** (`813f024`) | Obs-1/2 pinned by `no_lumotia_live_literal_target_in_live_rs`. Obs-4/5 pinned by `init_tracing_creates_log_file`. Obs-3 span propagation not directly tested; honour author's "everything else fans out from the 4 instrumented sites" SAFETY note. Real residual: storage crate emitted via `log::*!` macros (not `tracing::*!`), and no `tracing-log::LogTracer` bridge was installed — so events tagged `lumotia_storage` listed in `DEFAULT_STDERR_FILTER` never reached any layer. Migration progress + audio-cleanup warnings missing from `lumotia.log` forensic stream. Storage crate swapped from `log = "0.4"` to `tracing = "0.1"` (matching every other workspace crate); 4 call sites converted, 2 reformatted as structured tracing events. | +| **B.9** | JSON-envelope LLM extractor (GBNF removal) | `1d71e8e` | **Done** (`401b6c3`) | 7 existing tests cover the parser. Real residual: `extract_json_envelope_skips_qwen_thinking_prefix` only used an EMPTY `` block. Qwen3.5's reasoning mode emits non-empty content; if it contains JSON-looking text or unbalanced braces, the "find first '{' or '['" extractor either returns the reasoning literal or returns None (pollutes the brace-stack scanning past ``). Fix: strip the first `` before scanning. Falls back to whole text when no thinking-tag is present (covers non-reasoning models and the empty-thinking case). Two new regression tests for the JSON-in-thinking + unbalanced-braces-in-thinking cases. | +| **B.10** | `focusTimer` rehydrate `startTick` invariant | `5ba761a` (Race-10) | **Done** (`1c4ac98`) | The Race-10 fix was a comment-only "load-bearing comment" on the already-expired branch of `rehydrate()` saying "startTick() is REQUIRED here." The commit couldn't add a vitest test at the time because vitest scaffold landed the next day (Phase A.5 commit `206ac62`). Now that vitest is wired (jsdom env, `.svelte.ts` rune transformer, fake timers), the invariant is straightforwardly testable. New `src/lib/stores/focusTimer.test.ts` with one test `auto-clears the completion flash after the 3s window via the tick loop` that pins the contract — if a future edit drops the `startTick()` call, the test fails on the auto-clear assertion. | +| **B.11** | `FirstRunPage` unlisten on all exits | `6aa6a43` (Race-9) | **Documented pass** | No real residual. The Race-9 fix (let-declared handles outside try, finally guards each `if (h) h();`) is correctly applied and consistently mirrored across all multi-listen surfaces: `FirstRunPage.svelte`, `SettingsPage.svelte:874-902`, `FilesPage.svelte:25-44`. DownloadProgress payload shape matches the Rust emitter (`percent` field present in `DownloadProgress`). Hypothetical residuals considered + rejected: wrapping each `unlisten()` in try/catch (Tauri's UnlistenFn is a sync local-registry deregistration; throwing is implausible; would be the over-defensive-for-testability anti-pattern); extracting a unit-testable utility (over-refactor-for-testability anti-pattern). Component testing requires `@testing-library/svelte` — too heavy for one test. | +| **B.12** | `Transcriber::transcribe_sync_with_abort` required | `e0e9a6e` (Lifecycle-2) | **Documented pass** | No real residual. Three existing tests cover the trait-level requirement (`FlagSnoopingBackend`) + `SpeechModelAdapter` pre-dispatch short-circuit + dispatch-when-clear. The compile-time gate (no default impl) is the strongest guarantee; all 4 `impl Transcriber` sites (`SpeechModelAdapter`, `WhisperRsBackend`, `FlagSnoopingBackend`, `FakeTranscriber`) have explicit implementations. The "uncancellable middle" race in `SpeechModelAdapter` is explicitly documented with `// SAFETY:` + architectural reasoning — honour the author's annotation. | +| **B.13** | `drain_inference` deadline from `task.duration_secs` | `07f6755` (Time-bomb-1) | **Documented pass** | No real residual. Four existing tests pin every branch of `drain_timeout_for_inflight` (5x scaling, floor for short chunks, None fallback, NaN/inf/0/negative defensive). The drain loop's actual firing path is explicitly acknowledged via `// SAFETY:` at lines 557-561 as requiring a wedged whisper-rs that can't be fixtured. Honoured. | +| **B.14** | Surface capture-thread drops + bypass validation requeue cap | `094b533` | **Documented pass** | No real residual. `poll_capture_drops` arithmetic is correct (rollback-deferred-until-dimensions-known semantics, saturating-sub preserves the delta for re-observation, `.max(1)` guards sample_rate=0). `replay_buffer` consumer-side drain bypasses the 32-slot channel cap as the commit claimed. Unit-testing the full pipeline requires cpal hardware which can't be mocked without invasive refactoring — honour the implicit "this can't be fixtured cheaply" boundary. | +| **B.15** | `test_llm_model` respects caller GPU preference | `afbd33d` (Race-8) | **Documented pass** | No real residual. The fix itself is minimal + correct (`Option` parameter with `unwrap_or(true)` matching `load_llm_model`'s default; inline comment explains the (id, path, use_gpu) triple-mismatch race). Phase B.7's `AlreadyLoading` guard now provides additional protection — the second concurrent load is refused rather than silently overwritten. The remaining latent concern (`SettingsPage.svelte:694` doesn't pass `use_gpu` because the frontend has no UI for it yet) is a future feature-work item, not a B.15 residual. 8 existing classifier tests cover the error-categorisation surface. | ## Anti-patterns to avoid @@ -81,3 +81,120 @@ For each B.x: * `stop_live_transcription_session` comments + code consistent. No fix needed. **Verification:** `cargo test -p lumotia --lib commands::live` 17 / 17 (unchanged, comment-only edit); clippy + fmt clean. + +### B.2 — supervisor doc + test name match detach semantics (`643985d`) + +**Surface:** `crates/hotkey/src/supervisor.rs`. `SHUTDOWN_TIMEOUT` const doc and test `shutdown_force_aborts_stuck_tasks_after_timeout` both claimed "force-abort" semantics. The production code consumes the `JoinHandle` via `timeout(d, handle).await` — when the timeout fires, the inner future (the JoinHandle) is dropped. Dropping a JoinHandle DETACHES the task; it does NOT abort. The log message on the timeout branch already correctly says "detaching", and the `shutdown()` doc-comment says "detached and logged" — so the contract was always detach, but two satellite places said "abort". + +**Why it matters:** B.1-class hazard. A future maintainer trusting either "force-abort" claim would either add `handle.abort()` to make the implementation match (changing shutdown semantics — abort skips cooperative cleanup) or conclude the doc is wrong and need to retrace which is authoritative. + +**Verification:** `cargo test -p lumotia-hotkey --lib --tests` 6 unit + 2 integration = 8/8 pass; clippy + fmt clean. + +### B.3 — `download_impl` unlinks `.part` on ResumeUnsupported (`31e3f5a`) + +**Surface:** `crates/llm/src/model_manager.rs`. When a stale `.part` exists and the server returns 200 (full body) to a Range request, `download_impl` returned `DownloadError::ResumeUnsupported` without unlinking the `.part`. Every subsequent `download_model()` call sees the same `resume_from > 0`, sends the same Range request, gets the same 200 → wedged forever until the user calls `delete_model()`. + +**Why it matters:** Same reversibility-kill family as Rev-1: stale partial state stuck on disk, no automatic recovery, requires out-of-band intervention. + +**Fix:** `tokio::fs::remove_file(&tmp).await.ok()` before returning `ResumeUnsupported`. Single retry now recovers. New test `resume_unsupported_unlinks_part_so_retry_starts_fresh` with a server that ignores Range and returns 200. + +**Verification:** `cargo test -p lumotia-llm --lib model_manager` 5/5 pass; clippy + fmt clean. + +### B.4 — atomic `DELETE … RETURNING` in `purge_deleted_transcripts` (`20ef6c4`) + +**Surface:** `crates/storage/src/database.rs`. The prior form was a two-statement SELECT-then-DELETE-WHERE-id-IN pair. A `restore_transcript(id)` between (1) and (2) cleared `deleted_at` on a row in the chunk, but the DELETE had no `deleted_at IS NOT NULL` filter — so the now-LIVE row got hard-deleted alongside its audio file. + +**Why it matters:** Bypasses the entire Rev-2 soft-delete safety contract — the user can lose data without the 30-day retention window the contract promised. In current code the purge runs at startup before the user can issue a restore, so the race window is narrow in practice. Should be structural, not operational — especially if a future change moves the purge to a daily cron. + +**Fix:** single `DELETE FROM transcripts WHERE deleted_at IS NOT NULL AND deleted_at < datetime('now', ?) RETURNING audio_path`. SQLite evaluates the WHERE clause atomically with row removal; returned `audio_path`s are guaranteed to belong to rows this call hard-deleted. Also removes the chunking concern (no IN-clause, no SQLITE_MAX_VARIABLE_NUMBER ceiling). New test `purge_audio_cleanup_only_fires_for_hard_deleted_rows`. + +**Verification:** `cargo test -p lumotia-storage --lib database::tests` 53/53 pass; clippy + fmt clean. + +**Other finding (not fixed in this commit, recorded for triage):** UI components added by `87e6248` and `50d0715` carry stale `TODO(test): no Svelte component test framework wired in the repo (vitest not installed)` comments. Vitest landed in Phase A.5 (`206ac62`). Adding Svelte component tests for the Trash view + clearAll modal is real follow-up work but outside the per-item B.4 methodology. + +### B.5 — `resolve_export_path` follows symlink target before containment check (`d8fa4ff`) + +**Surface:** `src-tauri/src/commands/fs.rs`. Trust-1 canonicalised only the parent of the requested path because the file usually didn't exist yet. But if a symlink ALREADY existed at the target path pointing outside the allowlist, `tokio::fs::write` follows it on `File::create -> open(2)` and silently writes to the outside target. The path-string containment check passed because the SYMLINK lived inside the base, even though its target did not. + +**Why it matters:** Concrete bypass: a symlink at `~/Downloads/notes.md -> ~/.bashrc` (innocently created by the user, or planted via another vulnerability) lets a compromised webview overwrite shell startup via `write_text_file_cmd`. Trust-2 (audio.rs) already handled this for recordings via full-path canonicalize; Trust-1 had the asymmetric gap. + +**Fix:** two-mode canonicalisation in `resolve_export_path`. If the target exists → canonicalize the FULL path (follows symlink at the target before the containment check). On NotFound → fall back to parent canonicalize + join filename (the original save-dialog path). Two new symlink tests: `rejects_symlink_target_outside_allowlist` and `accepts_symlink_target_inside_allowlist` (both `#[cfg(unix)]`). + +**Verification:** `cargo test -p lumotia --lib commands::fs` 8/8 pass; clippy + fmt clean. + +### B.6 — capability JSON mirror invariant pinned (`7f0e1b0`) + +**Surface:** `src-tauri/src/commands/{clipboard.rs,paste.rs,security.rs}`. The `12b413d` commit message claimed the `CLIPBOARD_ALLOWED_WINDOWS` / `PASTE_REPLACING_ALLOWED_WINDOWS` Rust consts mirror the `windows` array in `capabilities/secondary-windows.json`, but no test pinned the relationship. A future change could add a new secondary window to the JSON but forget to update Rust (or vice versa, or typo a label) — silently drifting the IPC trust boundary against the capability grant. + +**Fix (test-only):** promoted both consts to `pub(crate)`, cross-linked their docstrings to the new test, and added `commands::security::tests_capability_mirror::allowlists_match_capability_jsons`. The test reads both `main.json` + `secondary-windows.json` at `CARGO_MANIFEST_DIR` and asserts every label in either Rust allowlist is declared in one of the JSONs' `windows` arrays. + +**Verification:** `cargo test -p lumotia --lib commands::security` 5/5 pass; clippy + fmt clean. + +### B.7 — `unload()` honours the `loading` flag (`f252c1b`) + +**Surface:** `crates/llm/src/lib.rs`. `unload()` did not consult the `loading` AtomicBool that `cde985d` introduced for Race-3/4 guard. Mid-load, `load_model_with`'s step 3 has already cleared `model` + `loaded`; a concurrent `unload()` takes the inner mutex, sees them already None, no-op-clears, returns Ok. Then step 5 installs the new state — the caller saw unload-success but the engine ends up loaded. + +**Why it matters:** Concrete scenario: app startup auto-loads default LLM in the background. User opens Settings, clicks "Delete Model X". `delete_llm_model` checks `loaded_model_id()` (returns None mid-load), skips the unload branch, calls `model_manager::delete_model(X)` → file deleted. The load completes via mmap (Linux inode held alive after unlink) and installs state pointing at a deleted file. UI shows "Model X loaded" even though the user just deleted it. + +**Fix:** `unload()` now checks `is_loading()` at entry, returns `EngineError::AlreadyLoading` when a load is mid-flight. Caller retries once `is_loading()` reports false. `EngineError::AlreadyLoading` message generalised to cover both directions. New test `unload_during_load_is_refused`. + +**Verification:** `cargo test -p lumotia-llm --lib` 26/26 pass; clippy + fmt clean. + +### B.8 — storage crate emits via `tracing` so events reach `lumotia.log` (`813f024`) + +**Surface:** `crates/storage/{Cargo.toml,src/database.rs,src/migrations.rs}`. `DEFAULT_STDERR_FILTER` + `DEFAULT_FILE_FILTER` both advertise `lumotia_storage=info` / `=debug` — operator intent: storage events surface in stderr AND the rolling `lumotia.log` forensic stream. Reality: storage used `log::*!` macros, no `tracing-log::LogTracer` bridge was installed, so every storage event vanished. Missing from diagnostic reports: migration progress on every schema bump, audio-cleanup warnings from `delete_transcript` + `purge_deleted_transcripts` (the two log lines Rev-3 specifically added). + +**Fix:** swap `log = "0.4"` for `tracing = "0.1"` in `crates/storage/Cargo.toml` (matching every other workspace crate). Convert the 4 call sites; reformat the two migration log lines as structured tracing events (version + description fields). + +**Verification:** `cargo test -p lumotia-storage --lib` 70/70 pass (unchanged); workspace clippy + fmt clean. + +### B.9 — strip leading `` reasoning before JSON-envelope scan (`401b6c3`) + +**Surface:** `crates/llm/src/lib.rs`. The `extract_json_envelope_skips_qwen_thinking_prefix` regression only covered an EMPTY `` block. Qwen3.5's reasoning mode emits non-empty content; if it contains JSON-looking literals (the model thinking out loud about schema), the naive "find first '{' or '['" extractor returned the reasoning's literal as the envelope. If the thinking contained unbalanced braces, the brace-stack scan pollutes past `` and the function returns None — losing the answer entirely. + +**Fix:** `text.split_once("")` and scan only the substring after. Falls back to whole text when no thinking tag is present (covers non-reasoning models AND the empty-thinking case). Backwards-compatible: empty thinking, no thinking, and trailing stop tokens all behave as before. Two new tests: `extract_json_envelope_skips_thinking_block_with_json_looking_content` and `extract_json_envelope_survives_unbalanced_braces_in_thinking`. + +**Verification:** `cargo test -p lumotia-llm --lib` 28/28 pass; clippy + fmt clean. + +### B.10 — vitest regression for `focusTimer` expired-rehydrate (`1c4ac98`) + +**Surface:** `src/lib/stores/focusTimer.test.ts` (new). The `5ba761a` commit added a load-bearing comment saying "startTick() is REQUIRED here" on the already-expired branch of `rehydrate()`. The commit couldn't add a test because vitest scaffold landed the day after (Phase A.5 — `206ac62`). With vitest now wired, the invariant is straightforwardly testable. + +**Fix:** new test seeds localStorage with an already-expired timer, calls `focusTimer.rehydrate()`, asserts the completion flash is visible, advances `vi.advanceTimersByTime(3_500)`, and asserts the flash auto-cleared (active=false, remainingMs=0, localStorage wiped). If a future edit removes the `startTick()` call, the tick loop won't run, the flash won't clear, and the test fires. + +**Verification:** `npm run test` 13/13 (was 12, +1 from this commit); `npm run check` 0 errors / 0 warnings across 4015 files. + +### B.11 — documented pass (no commit) + +**Surface:** `src/lib/pages/FirstRunPage.svelte`. The `6aa6a43` fix correctly applies the let-handles-outside-try + finally-guards pattern. Same pattern consistently mirrored across `SettingsPage.svelte:874-902` and `FilesPage.svelte:25-44`. DownloadProgress payload shape matches the Rust emitter. + +**Hypothetical residuals considered + rejected:** + +* Wrapping each `unlisten()` call in try/catch in case `unlisten()` itself throws — Tauri's `UnlistenFn` is a sync local-registry deregistration; throwing is implausible. Wrapping would be the over-defensive-for-testability anti-pattern flagged in the plan. +* Extracting a unit-testable listen-pair utility — over-refactor-for-testability anti-pattern. +* Adding `@testing-library/svelte` component tests — too heavy a dep for one component-shape test. + +### B.12 — documented pass (no commit) + +**Surface:** `crates/transcription/src/{transcriber.rs,local_engine.rs,whisper_rs_backend.rs}`. Three existing tests pin the trait-level contract (`FlagSnoopingBackend` + `SpeechModelAdapter` pre-dispatch + dispatch-when-clear). The compile-time gate (no default impl) is the strongest guarantee; all four `impl Transcriber` sites have explicit implementations. The "uncancellable middle" race in `SpeechModelAdapter` is documented with a `// SAFETY:` comment and architectural reasoning (live session drops receiver, orphan exits on `tx.send` failure) — honour the original author's annotation. + +### B.13 — documented pass (no commit) + +**Surface:** `src-tauri/src/commands/live.rs::drain_timeout_for_inflight`. Four existing tests cover every branch: 5x scaling, floor for short chunks, None fallback, defensive against NaN/inf/0/negative. The drain loop's actual firing path (set abort_flag, drop inflight, return error) is acknowledged as hard-to-fixture via the `// SAFETY:` comment at lines 557-561 (requires a wedged whisper-rs that can't be constructed without a real model). Honoured. + +### B.14 — documented pass (no commit) + +**Surface:** `crates/audio/src/capture.rs` + `src-tauri/src/commands/live.rs::poll_capture_drops` + `recv_audio`'s replay_buffer drain. The rollback-deferred-until-dimensions-known semantics correctly handle validation-window drops with saturating arithmetic. `replay_buffer` bypasses the 32-slot channel cap as the commit claimed. Unit-testing the full capture pipeline requires real cpal hardware which can't be mocked without invasive refactoring — honour the implicit "this can't be fixtured cheaply" boundary. + +### B.15 — documented pass (no commit) + +**Surface:** `src-tauri/src/commands/llm.rs::test_llm_model`. The `afbd33d` fix is minimal + correct (`Option use_gpu` with `unwrap_or(true)` matching `load_llm_model`'s default). Phase B.7's `AlreadyLoading` guard now provides defence-in-depth — a parallel load attempt is refused with `AlreadyLoading` rather than silently overwritten on a (id, path, use_gpu) triple mismatch. + +**Latent concern (not B.15-scope):** the frontend `SettingsPage.svelte:694` caller doesn't pass `use_gpu` because no UI for GPU on/off exists. When such UI is built, the test button should pass the user's preference through to `test_llm_model` (and `load_llm_model`). Future-feature work. + +### Phase B summary + +* Nine surgical commits in the audit pass: `643985d`, `31e3f5a`, `20ef6c4`, `d8fa4ff`, `7f0e1b0`, `f252c1b`, `813f024`, `401b6c3`, `1c4ac98` (B.2 through B.10). +* Five documented passes: B.11, B.12, B.13, B.14, B.15. +* New tests added: 8 Rust unit tests (B.2 rename, B.3 +1, B.4 +1, B.5 +2, B.6 +1, B.7 +1, B.9 +2) + 1 vitest test (B.10). +* All Phase A baseline gates remain green: `cargo test --workspace` 417 / 0, `cargo fmt --check` clean, `cargo clippy --workspace --all-targets -- -D warnings` clean, `npm run test` 13 / 13, `npm run check` 0 / 0 across 4015 files.