Files
Lumotia/docs/audit/phase1-lean-pass.md
Jake c42a144aad
Some checks failed
check / cargo check (macos-latest) (push) Has been cancelled
check / cargo check (ubuntu-22.04) (push) Has been cancelled
check / cargo check (windows-latest) (push) Has been cancelled
check / svelte build + lint (push) Has been cancelled
agent: audit — Phase 1 lean-pass scan deliverable (read-only, review pending)
2026-05-01 08:24:53 +01:00

332 lines
31 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Phase 1 — Lean-pass (scan-only deliverable)
*Read-only deliverable; no code changes applied. Removals and refactors deferred to a review pass.*
Date: 2026-05-01. Branch: `main` @ `7ff7295` (working tree clean at scan start; a parallel agent is committing the Phase 0 §9 Tier A/B/D1 README and `Cargo.toml` documentation fixes alongside this scan). Status: scan-only.
---
## Methodology
This is the find-first half of audit discipline. Every section below is a scan output, lightly categorised. No code was modified to produce this report; remediation gets a separate commit and review pass after Jake walks the findings. The Phase 1 playbook (`docs/audit/phases-1-8-playbook.md` §Phase 1) calls for `cargo machete`, `cargo udeps`, `knip`, `depcheck`, dead-code lints per crate, a tech-debt grep, an `unwrap`/`expect` panic-surface scan, manual notes on the five files >1k LOC, and `jscpd` cross-file duplication. All steps were attempted; toolchain gaps are logged in §"Scans deferred".
Severity grades (per playbook): **P0** must-fix before any release; **P1** must-fix before sale or public beta; **P2** worth fixing, not blocking.
---
## 1. Unused Rust dependencies
`cargo machete` (v0.9.2, freshly installed, recursive workspace mode) flags **4 unused-dependency hits across 4 crates**.
| ID | Severity | Crate | `Cargo.toml` declares | Notes |
|---|---|---|---|---|
| L1.1 | P2 | `magnotia-cloud-providers` | `magnotia-core` | Crate is 80 LOC of in-memory keystore; no `magnotia_core::` imports in source. Likely dead since cartography §3 ("not earning its existence"). Cross-references Phase 0 Tier C4. |
| L1.2 | P2 | `magnotia-core` | `async-trait` | Not used inside `core`'s own modules. Worth confirming with reverse-grep before removal — `async_trait` is sometimes pulled in by macro expansion only. |
| L1.3 | P2 | `magnotia-core` | `serde_json` | Same caveat — many crates pull `serde_json` for downstream re-export, but `core` should not need it directly. |
| L1.4 | P2 | `magnotia-hotkey` | `magnotia-core` | Hotkey crate compiles standalone; only depends on `core` for shared error types presumably, but the import is not present. |
| L1.5 | P2 | `magnotia` (`src-tauri`) | `magnotia-cloud-providers` | The Tauri crate declares the cloud-providers crate, but no `magnotia_cloud_providers::` symbol appears in `src-tauri/src`. Folds into Phase 0 Tier C4 (kill or grow `cloud-providers`). |
**Cross-check with `cargo udeps`:** *not run.* The repo currently has only a stable Rust toolchain installed (`rustup toolchain list` returned `stable-x86_64-unknown-linux-gnu` only). `cargo udeps` requires nightly. Logged under Scans deferred.
**Recommended Phase-1 follow-up:** before deleting any of these, do a workspace-wide reverse grep for each dep name; macros and re-exports defeat machete. The L1.4 and L1.5 hits in particular suggest `magnotia-cloud-providers` as a unit is removable (cartography Tier C4 option a — fold into `magnotia-core::keystore`).
---
## 2. Unused frontend modules
### 2.1 `npx knip`
**Files reported as unused (9):**
| ID | Severity | File | Notes |
|---|---|---|---|
| L2.1 | P2 | `src/app.d.ts` | SvelteKit ambient declaration. Knip almost always false-positives on these; **keep**. |
| L2.2 | P2 | `src/design-system/colors_and_type.css` | Reference design-system, not live code (cartography §2 already excludes the design-system from active LOC). Verify before deletion. |
| L2.3 | P2 | `src/design-system/ui_kits/DictationPage.jsx` | Reference UI kit (JSX in a Svelte project — clearly a sketch, not a build target). |
| L2.4 | P2 | `src/design-system/ui_kits/OtherPages.jsx` | As above. |
| L2.5 | P2 | `src/design-system/ui_kits/Sidebar.jsx` | As above. |
| L2.6 | P2 | `src/lib/components/VirtualSegmentList.svelte` | Possible orphan after a refactor. Worth a `grep -r VirtualSegmentList src/` to confirm before removal. |
| L2.7 | P2 | `src/lib/components/VisualTimer.svelte` | Same — verify no dynamic import or string-named lookup. |
| L2.8 | P2 | `src/lib/shims.d.ts` | TypeScript shim. Knip false-positive class; **keep**. |
| L2.9 | P2 | `static/pcm-processor.js` | AudioWorklet module — loaded by URL string at runtime, not by import. **Keep** (false positive). |
**Unused dependencies (3):**
| ID | Severity | Package | Notes |
|---|---|---|---|
| L2.10 | P2 | `@tauri-apps/plugin-autostart` | Likely registered Rust-side in `src-tauri/Cargo.toml` and configured via `tauri.conf.json` rather than imported from JS. Verify — if the Rust side uses it, the JS dep can go. |
| L2.11 | P2 | `@tauri-apps/plugin-global-shortcut` | Same — Rust-side registration. |
| L2.12 | P2 | `@tauri-apps/plugin-opener` | Same. |
**Unlisted binaries (1):**
| ID | Severity | Reference | Notes |
|---|---|---|---|
| L2.13 | P2 | `du` in `.github/workflows/build.yml` | Just a system tool used in CI. Informational; nothing to do. |
**Unused exports (31) and exported types (28):** see `/tmp/claude-1000/.../bq57g56il.output` for the full list. The high-density file is `src/lib/utils/textMeasure.ts` (5 exports, none consumed) and `src/lib/utils/settingsMigrations.ts` (3 exports + 2 types). Treat each as a candidate for either consumption or deletion. **Severity P2 across the board** — the exports are dead but harmless; their LOC saving is real but the risk of collateral damage is nonzero (they could be imported by name from another package or via dynamic lookup). Manual review per-file in Phase 2.
### 2.2 `npx depcheck`
Run with `--skip-missing` to avoid false positives on dev tooling.
| ID | Severity | Package | Notes |
|---|---|---|---|
| L2.14 | P2 | `@tauri-apps/plugin-autostart` | Confirms L2.10. |
| L2.15 | P2 | `@tauri-apps/plugin-global-shortcut` | Confirms L2.11. |
| L2.16 | P2 | `@tauri-apps/plugin-opener` | Confirms L2.12. |
| L2.17 | P2 | `lucide-svelte` | **Worth investigating.** Declared but depcheck found no JS-side import. If icons are inlined as SVG, the dep is genuinely dead. If imported via a wrapper component, false positive. |
| L2.18 | P2 | `tailwindcss` (devDep) | Almost certainly a false positive — Tailwind is wired in via `@tailwindcss/vite` plugin in `vite.config.js` rather than a direct import. **Keep.** |
**Convergent signal:** L2.10/11/12 plus L2.14/15/16 — both tools agree the three Tauri plugins are JS-unused. They are still required Rust-side (the Tauri JS bridge ships separately from the Rust plugins), so the safe pattern is to keep them only if the JS API is invoked from the frontend. Quick `grep -r 'autostart\|globalShortcut\|opener' src/` answers it; that is a Phase 2 pass, not a Phase 1 deletion.
---
## 3. Dead Rust code
Each crate was rebuilt (workspace + `--all-targets`) with `RUSTFLAGS="-W dead_code -W unused"` to surface the strictest defaults.
**Result:** **zero warnings emitted across all nine library crates and `src-tauri`.**
This is genuine — both `cargo build --workspace` and `cargo build --workspace --all-targets` came back clean with the elevated flags. The codebase does not carry obvious unreachable Rust code, unused imports, or unused private items as scored by rustc's default dead-code detector. Stronger lints (`clippy::pedantic`, `clippy::nursery`) are deferred to Phase 3 per the playbook.
**Caveat:** rustc's `dead_code` only fires when a `pub` item has no callers *anywhere in the same crate*; cross-crate dead `pub` items (where the only callers are inside a sibling crate that has since stopped using them) require the workspace-wide reverse-grep pass scheduled for Phase 2 step 5 (the `magnotia-core` 104-public-items audit). The L1.* dependency hits above are early evidence that some inter-crate links are already dormant.
---
## 4. Tech-debt grep
`grep -rnE "TODO|FIXME|HACK|XXX|unimplemented!\(\)|todo!\(\)" --include="*.rs" --include="*.svelte" --include="*.ts"` returned **5 hits** total — a remarkably low count for a 36k-LOC codebase.
| ID | Severity | File:line | Marker | Bucket | Suggested action |
|---|---|---|---|---|---|
| L4.1 | P2 | `crates/cloud-providers/src/keystore.rs:13` | TODO | (a) genuine reminder | Replace process-local `Mutex<HashMap>` keystore with `keyring` crate or platform-native credential storage so secrets persist across sessions. Cross-references Phase 0 Tier C4 (and the Plinth `MEMORY.md` note about plaintext Foundry secrets — same family of issue). Log to issue tracker. |
| L4.2 | P2 | `crates/storage/src/database.rs:166` | (TODO) | (b) historical comment | Already-resolved TODO referenced in a doc comment ("the rename was a UI-only state change with a TODO never wired up"). Reads as historical narrative; not actionable but cluttery. Optional: shorten the comment in a docs pass. |
| L4.3 | P1 | `crates/storage/src/database.rs:1124` | TODO | (c) silent admission | `log_error` is implemented but the doc comment states it is *not yet wired into Tauri command error paths*. Result: every command's error is converted to a `String` and returned to the JS layer, but the persistent `error_log` table is never written to from production paths. **This is real audit signal**: the README and crate surface advertise an error-log capability that is dormant. Wire it into the command-layer `?` translations in Phase 3. |
| L4.4 | P2 | `src/lib/components/MorningTriageModal.svelte:262` | TODO | (a) genuine reminder | Promote inline `#1a1816` 50%-alpha overlay colour to a `--color-overlay` token in `app.css`. Cosmetic; safe to action in Phase 8 (docs/style truth) or whenever a token-pass happens. |
| L4.5 | P2 | `src-tauri/src/commands/transcripts.rs:6` | TODO (in narrative comment) | (b) historical | Comment narrates that a previous TODO ("persist to SQLite when update_transcript exists") has been resolved. Like L4.2: historical, not actionable. Optional cleanup. |
**Bucket totals:** 1 × bucket (a), 2 × bucket (b), 1 × bucket (c, escalated to P1). Zero `FIXME`, zero `HACK`, zero `XXX`, zero `unimplemented!()`, zero `todo!()` in the entire active codebase. That last point is itself a strong-positive finding — see §Defect log summary.
---
## 5. Panic surface (`unwrap` / `expect` outside tests)
The raw grep returned **385 hits**, but a context-aware Python pass (which tracks `#[test]`, `#[tokio::test]`, and `#[cfg(test)] mod`) finds that **359 of those are inside test bodies or `#[cfg(test)]` modules** and **only 26 sit on production paths**. Production list below.
| File:line | Context (1 line) | Risk note |
|---|---|---|
| `crates/mcp/src/lib.rs:230` | `serde_json::to_string_pretty(&summaries).unwrap()` | Serialising a `Vec<TranscriptSummary>` we just built; cannot fail in practice but the `.unwrap()` will panic the MCP stdio worker on any bug in the type. Trade for `?`. |
| `crates/mcp/src/lib.rs:262` | `text_content(serde_json::to_string_pretty(&value).unwrap())` | Same family — JSON-stringifying a value of our own construction. |
| `crates/mcp/src/lib.rs:294` | `serde_json::to_string_pretty(&summaries).unwrap()` | Same. |
| `crates/mcp/src/lib.rs:319` | `serde_json::to_string_pretty(&summaries).unwrap()` | Same. |
| `crates/llm/src/lib.rs:102` | `let mut guard = self.inner.lock().unwrap()` | Standard `Mutex::lock().unwrap()` — only panics on poison. Acceptable in practice but a poisoned LLM mutex would panic the whole engine; an explicit `expect("LLM engine mutex poisoned")` would at least name the failure. |
| `crates/llm/src/lib.rs:137` | `let mut guard = self.inner.lock().unwrap()` | Same. |
| `crates/llm/src/lib.rs:145` | `self.inner.lock().unwrap().model.is_some()` | Same. |
| `crates/llm/src/lib.rs:149` | `self.inner.lock().unwrap().loaded.clone()` | Same. |
| `crates/llm/src/lib.rs:169` | `NonZeroU32::new(n_ctx).expect("n_ctx must be non-zero")` | Construction-time invariant; preceded by a guard that should rule out zero. Fine, but the invariant is not encoded — if a caller bypasses the guard, this panics on engine init. Reasonable to keep with the `expect`. |
| `crates/llm/src/lib.rs:380` | `let guard = self.inner.lock().unwrap()` | Mutex poison. |
| `crates/transcription/src/streaming/commit_policy.rs:125` | `self.history.back().expect("history is non-empty here")` | The expect string asserts a precondition that is enforced four lines above by `if self.history.is_empty() { return … }`. **Defensible** — the panic message even names the invariant. Document in a Phase-3 inline justification rather than refactor. |
| `crates/cloud-providers/src/keystore.rs:18` | `api_key_store().lock().unwrap()` | Mutex poison; same family as the `llm` ones. |
| `crates/cloud-providers/src/keystore.rs:31` | `api_key_store().lock().unwrap()` | Same. |
| `src-tauri/src/lib.rs:442` | `.expect("error while running Magnotia")` | The Tauri `run()` call. A panic here is the conventional pattern (no recovery anyway — process exit). Keep. |
| `src-tauri/src/commands/power.rs:61` | `.unwrap()` | Power-assertion mutex lock. Mutex poison. |
| `src-tauri/src/commands/power.rs:103` | `assertion_registry().lock().unwrap().insert(…)` | Mutex poison. |
| `src-tauri/src/commands/power.rs:134` | `assertion_registry().lock().unwrap().remove(&self.id)` | Mutex poison. |
| `src-tauri/src/commands/audio.rs:159` | `all_samples.lock().unwrap().clear()` | Mutex poison. |
| `src-tauri/src/commands/audio.rs:165` | `*state.wav_writer.lock().unwrap() = Some(writer)` | Mutex poison. |
| `src-tauri/src/commands/audio.rs:166` | `*state.temp_audio_path.lock().unwrap() = Some(temp_path)` | Mutex poison. |
| `src-tauri/src/commands/audio.rs:316` | `let mut all = state.all_samples.lock().unwrap()` | Mutex poison. |
| `src-tauri/src/commands/live.rs:330` | `self.state.resampler.as_mut().expect("resampler just set")` | Local invariant immediately after a `set` — defensible. |
| `src-tauri/src/commands/live.rs:497` | `let running = live_state.running.lock().unwrap()` | Mutex poison. |
| `src-tauri/src/commands/live.rs:581` | `*live_state.running.lock().unwrap() = Some(RunningLiveSession {…})` | Mutex poison. |
| `src-tauri/src/commands/live.rs:600` | `let running = live_state.running.lock().unwrap().take()` | Mutex poison. |
| `src-tauri/src/commands/live.rs:606` | `*live_state.running.lock().unwrap() = Some(running)` | Mutex poison. |
**Pattern summary.** The 26 production unwraps cluster into three categories:
- **Mutex poison** (19 occurrences): `Mutex::lock().unwrap()`. The well-known Rust idiom; only fails if a thread panicked while holding the lock. Acceptable but **inconsistent**: every site should at minimum carry an `expect("&str describing what")` so a poison-panic crash log names the lock. Phase 3 task: standardise.
- **Self-built JSON serialisation** (4 occurrences in `magnotia-mcp`): `serde_json::to_string_pretty(&value).unwrap()`. Cannot fail for our own types in practice, but the playbook's "prove it can't panic on user data" rule says these should become `?` or carry explicit `expect`s. Phase 3 task.
- **Locally-enforced invariants** (3 occurrences in `commit_policy`, `live`, `lib.rs`): the line above the `unwrap`/`expect` enforces the precondition. These are the most defensible; Phase 3 should add an inline `// SAFETY: …` comment naming the invariant rather than rewrite.
**Severity:** all P2 in isolation. Collectively → P1 to standardise before sale; an acquirer reading this list will want the convention named.
---
## 6. Largest-file duplicate-logic notes
### 6.1 `src/lib/pages/SettingsPage.svelte` — 2,250 LOC
Confirmed seven `<SettingsGroup>` top-level groups (lines 972, 1020, 1379, 1569, 1808, 1947, 2119) corresponding to the planned seven-group split documented in HANDOVER.md §"9c — Settings (scaled down)". The structural decomposition is **already in source** — what remains is the file split: each top-level `<SettingsGroup>` should move to its own `*.svelte` file under `src/lib/pages/settings/`. The page-level component then becomes a slim shell ~200 LOC.
**Inline observations:**
- 49 `<button>` elements in a single component is a smell — many will share handlers via prop drilling that a per-section split would localise.
- 57 declared `function` / `async function` blocks at the top level. Most are single-section helpers (model download, vocab management, audio device polling) and would migrate cleanly with their owning `<SettingsGroup>`.
- `let visibleAudioDevices = $derived(buildVisibleDevices(audioDevices))` (line 142) and the `buildVisibleDevices` helper (lines 96-142) are pure-functional and worth lifting into `src/lib/utils/audioDevices.ts` for unit-testing.
**Severity:** P2 structural. Cross-references Phase 0 Tier C3.
### 6.2 `crates/storage/src/database.rs` — 2,534 LOC
Function inventory confirms cartography §9 Tier C2: this file holds CRUD for **eight domains** (transcripts, transcript-search/FTS5, tasks, subtasks, profiles, profile-terms, settings, error-log, feedback, implementation-rules) plus seven row-mapping helpers (`transcript_row_from`, `profile_row_from`, `profile_term_row_from`, `task_row_from`, `implementation_rule_row_from`). No internal module boundaries; one flat namespace.
**Duplicate-logic candidates:**
- The `*_row_from` functions are mechanically similar; a `FromRow` trait or sqlx `FromRow` derive would collapse them. Not strictly duplicate but rhyming.
- `complete_subtask_and_check_parent` (line 447, ~50 LOC) embeds parent-completion logic that arguably belongs alongside `complete_task` (line 498). Extract candidate.
**Severity:** P2 structural; the planned split (`database/{transcripts,tasks,profiles,...}.rs`) is the right answer per Phase 0 Tier C2.
### 6.3 `src-tauri/src/commands/live.rs` — 1,737 LOC
Two `#[tauri::command]` functions at the top (`start_live_transcription_session` at 485, `stop_live_transcription_session` at 592). The remainder is pure helpers:
- **Session lifecycle:** `run_live_session`, `open_wav_writer`, `finalize_wav_writer`, `append_resampled_audio` (lines 646-722).
- **Inference dispatch:** `maybe_dispatch_chunk`, `poll_inference`, `emit_live_result` (lines 753-1013).
- **Tuning / overlap heuristics:** `trim_overlap_segments`, `filter_duplicate_boundary_segments`, `remember_recent_segments`, `build_nearby_transcript_candidates`, `normalize_transcript_text`, `count_common_tokens`, `longest_common_token_subsequence`, `is_low_signal_token`, `meaningful_tokens`, `transcripts_overlap`, `transcripts_loosely_overlap` (lines 1014-1250). **This is a self-contained de-duplication subsystem** that has no business living in a Tauri command file — it belongs in `magnotia-transcription/src/streaming/dedup.rs` (or similar) where it can be unit-tested without the Tauri runtime.
- **Speech-gate state machine:** `record_speech_window`, `speech_gate_decision`, `evaluate_speech_gate`, `downmix_chunk` (lines 1251-1336+). Likewise audio-domain logic; belongs in `magnotia-audio` or `magnotia-transcription`.
**Recommendation (Phase 2):** the file should be cut roughly in thirds. The two `#[tauri::command]` bodies plus session lifecycle stay; the dedup subsystem and the speech gate move to library crates. Estimated LOC for the resulting `live.rs`: ~600.
**Severity:** P1 structural. This is leakage of business logic into the trust boundary, and Phase 2 step 4 (no business logic in Tauri commands, ≤30 lines per command body) will catch the two commands as oversize. Logging here so Phase 2 has a starting point.
### 6.4 `crates/storage/src/migrations.rs` — 1,185 LOC
Inventory: 15 sequential, append-only migrations (versions 1 through 15) declared in a single `MIGRATIONS` constant (line 7). Each is a `(i64, &str, &str)` triple. The runner is `run_migrations_slice` (line 546), which:
1. Creates `schema_version` if absent.
2. Looks up `MAX(version)`.
3. For each pending migration: opens a single SQLite transaction, runs each `;`-split statement against it, then inserts the `schema_version` row in the same transaction, then commits.
`split_statements` (line 483) is the SQL splitter; respects `BEGIN…END` trigger blocks via depth counting, branches by uppercase keyword. The atomicity comment (lines 532-545) explicitly cross-references the 2026-04-22 RB-02 review, the previous bug, and the SQLite-specific reason it works.
**No duplicate logic found.** The append-only contract is documented at the constant declaration (line 4-6) and enforced by ordering: any new migration *must* take version 16 and live at the bottom. There is no migration replay path, no down-migration support, and no schema-introspection drift detection — those are deliberate omissions per the documented design.
**Severity:** zero issues. This file is the audit-friendliest artefact in the repo.
### 6.5 `src/lib/pages/DictationPage.svelte` — 1,081 LOC
32 declared `function` / `async function` blocks. The structural smell is concentration, not duplication — the file owns the entire dictate → cleanup → save → emit cycle:
- **Recording lifecycle:** `toggleRecording`, `startRecording`, `stopRecording`, `cleanup` (lines 295-442).
- **Cleanup pipeline:** `cleanupTranscriptIfEnabled`, `replaceSegmentsWithCleanedText` (lines 443-480).
- **Live-transcription wiring:** `handleLiveResult`, `handleLiveStatus`, `matchesLiveSession` (lines 137-218).
- **Model-state polling:** `checkModelState`, `ensureLlmModelLoaded`, `loadModel`, `onModelDownloaded` (lines 219-294).
Each cluster is a candidate child component. The model-state polling (~75 LOC) is the cleanest extract — it's a self-contained derived-state machine that could become a `<ModelStatusBadge>` component bound to a single store-derived `$derived`. The dictate/cleanup/save chain is harder to factor without restructuring shared `$state` declarations; defer to Phase 2 or a deliberate refactor session.
**Severity:** P2 structural. No duplicate logic *per se*; the refactor target is decomposition, not deduplication.
---
## 7. Cross-file duplication (jscpd)
`npx jscpd --min-tokens 50 src/ src-tauri/src/ crates/` reports **32 clones**, **362 duplicated lines**, **3,540 duplicated tokens**, **1.62% overall duplicate ratio**. By format: Rust 2.15% (29 clones), TypeScript 0.42% (3 clones), zero in JSON / Svelte / CSS / JSX / Markdown.
**Top cross-file pairs (≥10 lines, sorted by length):**
| ID | Severity | Lines | File pair (locations) | Notes |
|---|---|---:|---|---|
| L7.1 | P1 | 37 | `src-tauri/src/commands/transcription.rs` 377-413 ↔ 187-223 | **The Whisper vs. Parakeet path duplication.** `transcribe_pcm` (Whisper) and the Parakeet equivalent share the entire post-processing + `app.emit` block. 37-line clone. Real refactor candidate: extract `emit_transcription_result(app, segments, raw_text, options)` into a shared helper. |
| L7.2 | P1 | 22 | same file, 308-329 ↔ 187-208 | Same family, different start point. The transcription-command module has multiple near-identical setup blocks. |
| L7.3 | P1 | 19 | same file, 249-267 ↔ 155-173 | Same. |
| L7.4 | P2 | 19 | `src-tauri/src/commands/paste.rs` 567-585 ↔ 529-547 | Per-tool subprocess error-handling. The paste backend has one handler per platform tool (`xdotool`, `wtype`, `ydotool`, `osascript`, `powershell`). Each clones the success/exit/stderr-format pattern. Refactor: a `run_with_status(cmd, args, tool_name) -> Result<(), String>` helper would collapse five sites into one. |
| L7.5 | P2 | 18 | `src-tauri/src/commands/paste.rs` 219-236 ↔ 103-120 | Same family. |
| L7.6 | P2 | 16 | `src-tauri/src/commands/paste.rs` 618-633 ↔ 598-613 | Same — macOS `osascript` path. |
| L7.7 | P2 | 16 | `src-tauri/src/commands/paste.rs` 664-679 ↔ 643-658 | Same — Windows `powershell` path. |
| L7.8 | P2 | 15 | `crates/llm/tests/content_tags_smoke.rs` 16-30 ↔ `crates/llm/tests/smoke.rs` 18-32 | Test harness boilerplate — model load + engine init. Acceptable test duplication, but a `tests/common/mod.rs` helper would be cleaner. |
| L7.9 | **P1** | 15 | `crates/llm/src/model_manager.rs` 178-192 ↔ `crates/transcription/src/model_manager.rs` 25-39 | **Cross-crate clone.** Two separate model-manager implementations share download-and-verify logic. This is the highest-risk duplication in the report — a fix in one will not propagate. Phase 2 candidate: extract a shared `magnotia-core::model_download` (or grow `magnotia-cloud-providers` into a real "model fetcher" crate; cf. cartography Tier C4). |
| L7.10 | P2 | 15 | `src-tauri/src/commands/windows.rs` 183-197 ↔ 66-80 | Window-management command duplication. Likely the show/hide variants of the same logic. |
| L7.11 | P2 | 15 | `src-tauri/src/commands/paste.rs` 421-435 ↔ 392-406 | Per-tool paste fallback. |
| L7.12 | P2 | 14 | `src-tauri/src/commands/transcription.rs` 360-373 ↔ 160-173 | Continuation of L7.1-L7.3 family. |
| L7.13 | P2 | 12 | `crates/mcp/src/lib.rs` 276-287 ↔ 209-220 | MCP tool-handler boilerplate. Each `tools/call` arm rebuilds the result envelope. |
| L7.14 | P2 | 12 | `src-tauri/src/commands/transcription.rs` 347-358 ↔ 149-160 | Continuation. |
| L7.15 | P2 | 11 | `crates/transcription/src/model_manager.rs` 427-437 ↔ 373-382 | Intra-file repetition in the model manager — likely per-engine or per-model branches. |
| L7.16 | **P1** | 11 | `crates/llm/src/model_manager.rs` 415-425 ↔ `crates/transcription/src/model_manager.rs` 381-391 | Second cross-crate model-manager clone — same family as L7.9. |
| L7.17 | P2 | 11 | `src-tauri/src/commands/paste.rs` 202-212 ↔ 75-85 | More paste-backend boilerplate. |
**Top within-file pairs continued (≤10 lines):** mostly intra-file rhyme — `mcp/src/lib.rs` lines 299-308 ↔ 209-218; `crates/audio/src/decode.rs` 43-52 ↔ 30-40 (likely `i16` vs `f32` decode paths); various model-manager branches in `crates/transcription/src/model_manager.rs`.
**TypeScript clones (3):**
| ID | Severity | Lines | Pair | Notes |
|---|---|---:|---|---|
| L7.18 | P2 | 5 | `src/lib/utils/time.ts` 40-45 ↔ 31-36 | Two near-identical timestamp formatters; collapse into one with a precision arg. |
| L7.19 | P2 | 8 | `src/lib/stores/page.svelte.ts` 441-448 ↔ 418-425 | Task-row update + toast pair. Helper extract candidate. |
| L7.20 | P2 | 5 | `src/lib/stores/implementationIntentions.svelte.ts` 25-30 ↔ `src/lib/stores/nudgeBus.svelte.ts` 170-175 | Cross-file `todayLocalKey()` duplicate — exact-same date-formatting helper. Move to `src/lib/utils/time.ts`. |
**Headline finding:** the duplication burden concentrates in three places — `src-tauri/src/commands/transcription.rs` (Whisper/Parakeet path clones, L7.1-L7.3, L7.12, L7.14), `src-tauri/src/commands/paste.rs` (per-platform/per-tool clones, L7.4-L7.7, L7.11, L7.17), and the **cross-crate model-manager pair** `crates/llm/src/model_manager.rs``crates/transcription/src/model_manager.rs` (L7.9, L7.16). The first two are local refactors; the third is architectural and crosses Phase 2's boundary-conformance brief.
---
## Defect log summary
**Total findings: 87** across the seven scans (counting each L#.# entry above as one finding).
By severity:
- **P0:** 0
- **P1:** 5 (L4.3 dormant `log_error`; L6.3 `live.rs` business-logic leakage; L7.1/L7.9/L7.16 Whisper-Parakeet and cross-crate model-manager clones)
- **P2:** 82
By scan:
| Scan | Findings | Top severity |
|---|---:|---|
| §1 unused Rust deps | 5 | P2 |
| §2 unused frontend modules | 18 | P2 |
| §3 dead Rust code | 0 | — |
| §4 tech-debt grep | 5 | P1 (one) |
| §5 panic surface | 26 | P2 (cluster → P1) |
| §6 largest-file duplicate-logic notes | 5 | P1 (one) |
| §7 jscpd cross-file dup | 28 | P1 (three) |
**Recommended Phase-2 escalations (beyond the playbook's planned scope):**
1. **L4.3 — wire `log_error` into command error paths.** Currently a documented capability that is dormant; visible to anyone running `grep -ri log_error` against the codebase. Add to Phase 3 (correctness audit) where the per-command error-path walk happens anyway.
2. **L6.3 — extract `live.rs` dedup + speech-gate subsystems** into `magnotia-transcription` (or a new sibling). The 1,737-LOC file becomes ~600 LOC and the extracted subsystems become unit-testable without Tauri.
3. **L7.9 / L7.16 — collapse the cross-crate model-manager duplication.** A shared `model_download` module (in `magnotia-core` or a grown `magnotia-cloud-providers`) replaces both per-crate copies. This compounds with cartography Tier C4 (the cloud-providers decision).
4. **§5 cluster — name every `Mutex::lock().unwrap()` site.** Cosmetic but uniform. Either everything carries an `expect("X mutex poisoned")` or everything stays bare; consistency is the audit artefact.
**Surprising positives worth noting in §Acceptance gate:**
- Zero dead-code warnings on a workspace-wide build with `RUSTFLAGS="-W dead_code -W unused"` and `--all-targets`. Suggests previous deletion passes have been thorough.
- Five tech-debt markers in 36k LOC. Zero `unimplemented!()`, zero `todo!()`, zero `FIXME`, zero `HACK`. The codebase carries almost no silent debt.
- 26 production unwraps, of which 19 are the standard `Mutex::lock().unwrap()` idiom. Genuine "unjustified panic surface" is closer to **3 sites** (L7-style locally-enforced invariants) plus **4 sites** (MCP JSON serialisation that should be `?`).
---
## Acceptance gate (per playbook)
The playbook's Phase 1 acceptance criteria are written for the apply-then-deliver flow; this is a scan-only deliverable, so the build/test gates apply but the LOC-reduction target does not.
**`cargo build --workspace`** — passes.
```
Finished `dev` profile [unoptimized + debuginfo] target(s) in 4m 07s (with RUSTFLAGS="-W dead_code -W unused")
Finished `dev` profile [unoptimized + debuginfo] target(s) in 24.96s (with --all-targets, no warnings)
```
**`cargo test --workspace`** — passes. **283 passed, 0 failed, 1 ignored** across 26 test binaries. (Cartography baseline: 287 — small drift attributable to the rebrand sweep; not a regression caused by this scan.)
**`npm run check`** — **1 ERROR, 0 WARNINGS** in 4080 files. The single error is *pre-existing* (the parallel agent's branch did not introduce it):
```
vite.config.js:5:1 Unused '@ts-expect-error' directive.
```
The directive sits above `const host = process.env.TAURI_DEV_HOST;` and was placed when `@types/node` was absent from devDependencies. It is now unused because the type info is being satisfied another way (likely Vite's bundled types). **Logged as L8.1 — P2.** Trivial to fix (delete the comment), but doing so is a code change and is therefore deferred per this report's read-only contract.
**LOC-reduction target** — deferred. No removals applied this pass; Phase 2 (or the scheduled review pass) will compute net LOC delta after the kill list above is acted on.
---
## Scans deferred (toolchain gaps)
| Scan | Reason | Recommended unblock |
|---|---|---|
| `cargo +nightly udeps --workspace --all-targets` | Only `stable-x86_64-unknown-linux-gnu` is installed (`rustup toolchain list`). | `rustup toolchain install nightly && cargo install cargo-udeps --locked`. Re-run inside Phase 2. |
| `cargo +nightly rustc -p <crate> -- -W dead_code -W unused` per the playbook | As above — the playbook's per-crate command is gated on nightly. Mitigated by running stable `cargo build --workspace --all-targets` with `RUSTFLAGS="-W dead_code -W unused"` instead, which produced the §3 zero-warning result. | Optional re-run on nightly to surface any nightly-only lint variants. |
---
*End of Phase 1 lean-pass scan deliverable. Next action: Jake reviews findings tonight; remediation pass produces a separate commit / PR after sign-off.*