Lumotia/docs/code-review-2026-04-22.md

name, description, type, tags, date

name	description	type	tags	date
Code Review — 2026/04/22	Full-sweep audit findings across all Kon crates + src-tauri, with triage buckets for quick wins vs release-blockers	reference	code-review audit bugs kon release-blockers	2026/04/22

Kon Code Review — 2026/04/22

Full-sweep read-only audit of every .rs file across the Kon workspace. Four parallel Codex agents scanned:

• Agent A — crates/transcription/, crates/audio/
• Agent B — crates/ai-formatting/, crates/llm/, crates/storage/
• Agent C — src-tauri/src/ (commands layer + lib.rs + main.rs + types.rs)
• Agent D — crates/core/, crates/cloud-providers/, crates/hotkey/, crates/mcp/, src-tauri/build.rs

Summary

Severity	Count
CRITICAL	4
MAJOR	16
MINOR	15
NIT	3

CRITICAL items are all real bugs — not speculative. Three were introduced or touched during the whisper-ecosystem sprint; one is a latent data-integrity issue in the storage layer.

Recommended path:

1. Fix the four CRITICALs this session.
2. Log all MAJORs as release-blockers (must land before v0.1).
3. MINORs become a boy-scout backlog — picked up opportunistically when adjacent code is touched.
4. NITs resolve inline when the surrounding file is next edited.

---

CRITICAL

C1 — Racy single-session guard in live.rs

• Path: src-tauri/src/commands/live.rs:193-338
• Issue: start_live_transcription_session checks running is None before multiple awaits and only stores the handle at the end; stop_live_transcription_session removes running before awaiting the worker join. Two overlapping IPC calls can admit a second live session OR expose an empty slot while the first session is still shutting down.
• Fix scope: large — requires holding the mutex across the async boundary or restructuring the state machine.
• Bucket: RELEASE-BLOCKER (this is the file's core invariant).

C2 — RmsVadChunker::flush() drops chunks

• Path: crates/transcription/src/streaming/rms_vad.rs:294-311
• Issue: flush() zero-pads the final partial frame and calls consume_frame() via let _ = ..., discarding the returned VadChunk. If the padded frame triggers end-of-utterance or max_chunk_samples, the emitted chunk is lost and the outer state check either returns None or an empty chunk.
• Fix scope: small — change flush trait signature to return Vec<VadChunk>, collect chunks from both the consume_frame call and the final emit_active_chunk_and_close.
• Bucket: QUICK WIN. Regression test in the same commit.
• Attribution: Introduced in 05eea41 yesterday.

C3 — Multi-statement migrations can half-apply

• Path: crates/storage/src/migrations.rs:263-299
• Issue: run_migrations executes statements individually and only records schema version after the full migration succeeds. A crash mid-migration leaves the schema half-mutated while still appearing unapplied; the next startup replays it against the partially-mutated DB.
• Fix scope: medium — wrap each migration in BEGIN/COMMIT transaction, update version row within the same transaction.
• Bucket: RELEASE-BLOCKER. A user with a mid-migration crash today gets a bricked DB.

C4 — Transcript provenance can reference deleted profiles

• Path: crates/storage/src/migrations.rs:208-216, crates/storage/src/database.rs:61-89, :697-708
• Issue: v8 migration adds transcripts.profile_id without a foreign-key constraint. insert_transcript accepts any profile_id; delete_profile doesn't guard against existing transcript references. Transcripts can keep orphaned profile IDs, breaking provenance integrity.
• Fix scope: large — v9 migration to add FK constraint + reconcile existing orphans; update delete_profile to either cascade or block.
• Bucket: RELEASE-BLOCKER. Silent data-integrity hole.

---

MAJOR (16)

src-tauri — Commands layer

[MAJOR] poll_inference treats IPC listener loss as session-fatal

• src-tauri/src/commands/live.rs:721-813
• Closing the frontend or reloading it kills the whole live session via ? on result_channel.send(...). Non-fatal Tauri channel lifecycle should not terminate capture.
• Fix scope: medium. Bucket: RELEASE-BLOCKER.

[MAJOR] run_live_session is a 200+ line multi-responsibility monolith

• src-tauri/src/commands/live.rs:349-579
• Owns mic startup, runtime error draining, resampling, progressive WAV persistence, overload dropping, inference scheduling, and shutdown finalisation in one function. Known lifecycle bugs trace to this.
• Fix scope: large. Bucket: RELEASE-BLOCKER (refactor enables C1 fix).

[MAJOR] Native capture worker is detached and can outlive stop/start

• src-tauri/src/commands/audio.rs:46-228
• start_native_capture spawns a worker but never retains a join handle. A previous capture can flush into all_samples after stop_native_capture clears it — truncation and cross-session contamination possible.
• Fix scope: medium. Bucket: RELEASE-BLOCKER.

[MAJOR] resolve_recording_path collides within the same second

• src-tauri/src/commands/audio.rs:236-257
• Filename derived from SystemTime::now().as_secs(). Two recordings started in the same second get the same path → overwrite or merge.
• Fix scope: small. Bucket: QUICK WIN (append milliseconds + session_id).

[MAJOR] get_runtime_capabilities advertises wrong accelerators

• src-tauri/src/commands/models.rs:435-489
• Hard-codes accelerators = ["cpu", "vulkan"] even when detect_active_compute_device would report metal on macOS or the binary was compiled without the whisper feature.
• Fix scope: medium. Bucket: RELEASE-BLOCKER (frontend shows wrong settings otherwise).

[MAJOR] paste_text_replacing doesn't snapshot the clipboard

• src-tauri/src/commands/paste.rs:181-217
• Inconsistent with paste_text. Replacing leaves the raw transcript on the clipboard and destroys whatever the user had copied before.
• Fix scope: small. Bucket: QUICK WIN.

[MAJOR] PowerAssertion::begin is a non-functional macOS stub

• src-tauri/src/commands/power.rs:41-121
• begin_activity always returns Err → guard never acquires an App Nap assertion. The plan for A.1 #9 explicitly deferred this; still flagging so it's not forgotten.
• Fix scope: medium. Bucket: RELEASE-BLOCKER (before macOS ship).

Transcription + audio

[MAJOR] Decoder returns partial audio on errors

• crates/audio/src/decode.rs:58-79
• Packet-read errors break the loop; decoder errors are skipped; function still returns Ok if any samples were produced. Truncated files silently accepted.
• Fix scope: medium. Bucket: RELEASE-BLOCKER.

[MAJOR] read_wav() silently drops sample decode errors

• crates/audio/src/wav.rs:135-145
• filter_map(|s| s.ok()) for both integer and float iterators. Corrupt samples silently discarded.
• Fix scope: small. Bucket: QUICK WIN.

[MAJOR] Model downloads don't validate non-resume HTTP status

• crates/transcription/src/model_manager.rs:161-262
• Resume branch checks 206/200. Normal downloads never call error_for_status() → a 4xx/5xx response body gets written to .part and renamed.
• Fix scope: small. Bucket: QUICK WIN.

LLM + storage

[MAJOR] LLM prompts not preflighted against context window

• crates/llm/src/lib.rs:143-166, :317-321
• generate tokenises the full prompt; context_window_size hard-caps at 8192. Long transcripts reach inference with prompts bigger than context → late runtime failure.
• Fix scope: medium. Bucket: RELEASE-BLOCKER.

[MAJOR] uncomplete_task doesn't reopen auto-completed parents

• crates/storage/src/database.rs:389-449
• complete_subtask_and_check_parent auto-completes a parent when the last child completes. uncomplete_task only flips the requested row → reopening a child leaves the parent wrongly marked done.
• Fix scope: small. Bucket: QUICK WIN.

Core + small crates

[MAJOR] keystore::store_api_key is a thread-unsafe safe API

• crates/cloud-providers/src/keystore.rs:6-18
• std::env::set_var is UB outside single-threaded init per documented precondition. The safe pub fn doesn't enforce this.
• Fix scope: medium. Bucket: RELEASE-BLOCKER.

[MAJOR] Hotkey device filtering hard-codes KEY_A / KEY_R

• crates/hotkey/src/linux.rs:236-241
• try_attach_device claims to check for the configured hotkey's key but tests for hard-coded KEY_A or KEY_R. Hotkeys on other keys get silently dropped.
• Fix scope: small. Bucket: RELEASE-BLOCKER (correctness bug in a feature users rely on).

[MAJOR] Malformed JSON-RPC silently dropped

• crates/mcp/src/main.rs:26-30
• stdio entry point logs malformed lines and moves on without sending a JSON-RPC parse-error response. handle_message has parse-error handling that never runs.
• Fix scope: small. Bucket: QUICK WIN.

[MAJOR] list_transcripts accepts invalid params as defaults

• crates/mcp/src/lib.rs:188-195
• serde_json::from_value(args).unwrap_or_default() converts malformed args into defaults. Every other handler in the file returns -32602 instead. Inconsistent behaviour.
• Fix scope: small. Bucket: QUICK WIN.

[MAJOR] CSP guard matches connect-src by prefix

• src-tauri/build.rs:47-64
• strip_prefix("connect-src") would also match connect-src-elem (if ever added to CSP3). Defensive: exact directive name match.
• Fix scope: small. Bucket: QUICK WIN.
• Attribution: Introduced in 6fd3893 yesterday.

---

MINOR (15)

Grouped here for brevity — full details in agent outputs. Bucket: BOY SCOUT (fix when adjacent code touched).

• commands/live.rs:341-347 — pick_engine duplicates dispatch logic from commands/models.rs and commands/transcription.rs
• commands/live.rs:123-145 — stale #[allow(dead_code)] on LiveStatusMessage (all variants are constructed)
• crates/audio/src/capture.rs:355-499 — open_and_validate() is 145 lines; only one unit test in the file
• crates/audio/src/lib.rs:14 + vad.rs:14-34 — SpeechDetector re-exported but no in-repo uses (stub awaiting Silero)
• crates/audio/src/resample.rs:25-39 + streaming_resample.rs:63-80 — rubato tuning duplicated between offline and streaming
• crates/transcription/src/local_engine.rs:83-157 — load/unload/capabilities/transcribe_sync have no direct tests
• crates/transcription/src/whisper_rs_backend.rs:54-107 — multi-responsibility function, behaviour-testing limited to Display
• crates/ai-formatting/src/pipeline.rs:38-100 — post_process_segments does filtering + formatting + LLM invocation + failure handling in one function
• crates/storage/src/database.rs (×4 sites) — repeated SELECT column lists invite schema drift
• crates/storage/src/database.rs (×3 sites) — list_transcripts_paged, count_transcripts, update_transcript, uncomplete_task, log_error, list_recent_errors all untested
• crates/storage/src/database.rs:774-775 — TODO flagging that Tauri command failures aren't wired into error_log
• crates/core/src/providers.rs:35-40 — dead ProviderRegistry suppressed with #[allow(dead_code)]
• crates/core/src/types.rs:169-184 — dead TranscriptMetadata suppressed with #[allow(dead_code)]
• crates/hotkey/src/lib.rs:44-77 — parser silently discards extra triggers (Ctrl+A+B parses as B); no malformed-combo tests
• crates/hotkey/src/linux.rs:46-142 — EvdevHotkeyListener::start is ~100 lines mixing channel setup + device scanning + watcher + retry + task orchestration
• crates/mcp/src/lib.rs:168-303 — list_transcripts, get_transcript, search_transcripts, list_tasks handlers untested

---

NIT (3)

• crates/ai-formatting/src/llm_client.rs:26-27, :59-60 — #[allow(dead_code)] on actively-used CLEANUP_PROMPT and format_dictionary_suffix
• crates/storage/src/file_storage.rs:12-14 — open TODO for consolidating OS-path helpers
• src-tauri/src/commands/live.rs:123-145 — covered above (re-flagged by Agent C as NIT)

---

Triage buckets

Quick wins (this session or next)

One concern per commit. TDD where testable — failing regression test, then fix.

1. C2 flush() drops chunks → change return type to Vec<VadChunk>
2. paste_text_replacing clipboard snapshot
3. resolve_recording_path collision → append millis + session_id
4. read_wav propagate sample errors
5. model_manager check HTTP status on non-resume path
6. uncomplete_task reopen auto-completed parents
7. CSP guard exact-name directive match (Rule: my own commit, Boy Scout)
8. MCP parse-error reply on malformed JSON-RPC
9. list_transcripts return -32602 on invalid params
10. Dead-code cleanups: ProviderRegistry, TranscriptMetadata, CLEANUP_PROMPT/format_dictionary_suffix allows, LiveStatusMessage allow

That's 10 items, ~1 commit each. Maybe 2–3 hours.

Release-blockers (before v0.1 ship)

Tracked items that must land before first public release:

• C1 racy single-session guard — needs run_live_session refactor first
• C3 migrations atomicity — BEGIN/COMMIT wrap + version in same tx
• C4 transcript-profile FK + delete_profile guard (v9 migration)
• run_live_session monolith refactor (unblocks C1)
• poll_inference IPC channel loss resilience
• Native capture worker join handle
• get_runtime_capabilities accelerator correctness
• PowerAssertion macOS objc2 bridge (known deferred)
• Decoder error propagation (audio/src/decode.rs)
• LLM prompt preflight against context window
• Keystore thread-safety
• Hotkey linux device filtering KEY_A/KEY_R bug

Boy Scout backlog

All MINORs + NITs. Pick up opportunistically when adjacent code is touched.

Deferred (quality improvements, not release-blocking)

• SQL SELECT list refactoring (needs macro or typed query builder)
• Test coverage improvements across local_engine, whisper_rs_backend, pipeline, storage APIs, MCP handlers
• Resampler tuning consolidation
• File-storage path helpers consolidation

---

Notes

• No TODO / FIXME / HACK / XXX markers in the transcription + audio crates (Agent A confirmed).
• Clean files: transcription/src/lib.rs, transcriber.rs, concurrency.rs, streaming/buffer_trim.rs, streaming/commit_policy.rs, streaming/mod.rs, audio/src/concurrency.rs, ai-formatting/src/{correction_learning,lib,rule_based,to_plain_text}.rs, llm/src/{grammars,prompts}.rs, storage/src/lib.rs.
• Most-touched files in the sprint (streaming/*, wav.rs, commit_policy, buffer_trim) came back clean from A and B — the sprint code itself is in reasonable shape; the bugs cluster in live.rs and older storage surfaces.