Land release blocker fixes and workspace cleanup
This commit is contained in:
@@ -11,43 +11,36 @@ Issues here must land before Kon v0.1 ships. Each is sourced from
|
||||
`docs/code-review-2026-04-22.md`. When `gh` CLI is available, these
|
||||
should be mirrored as real GitHub issues on `jakejars/kon`.
|
||||
|
||||
## CRITICAL (2 open, 1 resolved)
|
||||
## CRITICAL (0 open, 3 resolved)
|
||||
|
||||
No open CRITICAL blockers.
|
||||
|
||||
## MAJOR (1 open, 8 resolved)
|
||||
|
||||
| # | File | Area | Fix scope |
|
||||
|---|---|---|---|
|
||||
| RB-01 | [c1-live-session-race.md](c1-live-session-race.md) | `src-tauri/commands/live.rs` | large |
|
||||
| RB-03 | [c4-transcript-profile-fk.md](c4-transcript-profile-fk.md) | `crates/storage/migrations.rs` + `database.rs` | large |
|
||||
|
||||
## MAJOR (5 open, 4 resolved)
|
||||
|
||||
| # | File | Area | Fix scope |
|
||||
|---|---|---|---|
|
||||
| RB-04 | [run-live-session-monolith.md](run-live-session-monolith.md) | `src-tauri/commands/live.rs` | large |
|
||||
| RB-05 | [poll-inference-channel-fatality.md](poll-inference-channel-fatality.md) | `src-tauri/commands/live.rs` | medium |
|
||||
| RB-08 | [power-assertion-macos-objc2.md](power-assertion-macos-objc2.md) | `src-tauri/commands/power.rs` | medium |
|
||||
| RB-10 | [llm-prompt-preflight.md](llm-prompt-preflight.md) | `crates/llm/lib.rs` | medium |
|
||||
| RB-11 | [keystore-thread-safety.md](keystore-thread-safety.md) | `crates/cloud-providers/keystore.rs` | medium |
|
||||
|
||||
## Resolved
|
||||
|
||||
| # | File | Area | Resolution |
|
||||
|---|---|---|---|
|
||||
| RB-01 | [c1-live-session-race.md](c1-live-session-race.md) | `src-tauri/commands/live.rs` | Added `LiveTranscriptionState.lifecycle: tokio::sync::Mutex<()>` and hold it across the async spans of both `start_live_transcription_session` and `stop_live_transcription_session`. The running-slot check/insert and stop/take/join sequence are now serialized, so concurrent starts can no longer both pass the empty-slot check and a start during stop blocks until the previous worker fully joins. Two async regression tests cover both races. |
|
||||
| RB-02 | [c3-migrations-atomicity.md](c3-migrations-atomicity.md) | `crates/storage/migrations.rs` | Each migration now runs inside a `pool.begin()` / `tx.commit()` transaction alongside its `schema_version` insert. Regression test injects a poisoned v9 migration and asserts neither the partial schema change nor the version row persists. DRY'd `run_migrations_up_to` test helper onto the same code path. |
|
||||
| RB-03 | [c4-transcript-profile-fk.md](c4-transcript-profile-fk.md) | `crates/storage/migrations.rs` + `database.rs` | Added a transactional v9 rebuild of `transcripts` that enforces `profile_id REFERENCES profiles(id) ON DELETE RESTRICT`, reassigns any orphaned transcript provenance to `DEFAULT_PROFILE_ID`, rebuilds dependent `segments` / FTS state, and preserves valid profile references. `insert_transcript` now rejects unknown profile ids up front, and `delete_profile` returns a clear reassign-first error when transcripts still reference the profile. Regression tests cover migration reconciliation, invalid inserts, and delete rejection. |
|
||||
| RB-04 | [run-live-session-monolith.md](run-live-session-monolith.md) | `src-tauri/commands/live.rs` | Replaced the 200+ line `run_live_session` loop with an explicit `LiveSessionRuntime` + `LiveLoopState` structure. Capture startup, runtime mic-error draining, audio chunk processing, overflow handling, stop-tail flush, inference dispatch/drain, and WAV finalisation each live in focused helpers, preserving behaviour while making the lifecycle auditable enough for RB-01 follow-up. Existing live tests and the full `kon` lib suite stay green. |
|
||||
| RB-05 | [poll-inference-channel-fatality.md](poll-inference-channel-fatality.md) | `src-tauri/commands/live.rs` | `poll_inference` now treats result-channel loss as a listener-lifecycle problem rather than a transcription failure. On the first `result_channel.send(...)` error it marks the live result listener as lost, emits a single warning that transcription will continue in the background, and keeps processing later chunks without retrying the dead channel. Regression test simulates a dead result listener and asserts chunk processing continues with only one warning. |
|
||||
| RB-06 | [native-capture-worker-join.md](native-capture-worker-join.md) | `src-tauri/commands/audio.rs` | `NativeCaptureState.stop_tx` replaced by `worker: AsyncMutex<Option<CaptureWorker>>`. `CaptureWorker` bundles the stop sender and the spawned task's `JoinHandle`; `stop_worker(worker)` sends stop then `await`s termination. Both `start_native_capture` (prior-worker stop) and `stop_native_capture` use the helper. Removed the 50ms sleep — the join barrier is exact. Two regression tests cover the lifecycle guarantee and the already-exited case. |
|
||||
| RB-07 | [runtime-capabilities-accelerators.md](runtime-capabilities-accelerators.md) | `src-tauri/commands/models.rs` | Introduced `compose_accelerators(whisper_enabled, loader_available, target)` as a pure helper; `supported_accelerators()` reads `cfg(feature = "whisper")`, `vulkan_loader_available()`, and target OS then delegates. `get_runtime_capabilities` uses it in place of the hard-coded `["cpu", "vulkan"]`. Whisper's `supports_gpu` now follows the feature flag. Five regression tests cover all permutations. |
|
||||
| RB-09 | [decoder-partial-audio-on-error.md](decoder-partial-audio-on-error.md) | `crates/audio/decode.rs` | Packet-loop now propagates all non-EOF `SymphoniaError`s as `AudioDecodeFailed`; per-packet decode errors bubble via `?`. Mock-`MediaSource` regression test confirms mid-stream I/O errors surface instead of returning partial audio. |
|
||||
| RB-10 | [llm-prompt-preflight.md](llm-prompt-preflight.md) | `crates/llm/lib.rs` | Added an explicit prompt-budget preflight before context creation. If `prompt_tokens + max_tokens + reserve` exceeds the 8192-token cap, `generate` now returns a typed `EngineError::PromptTooLong { ... }` instead of failing late inside inference. Regression tests cover both the over-budget and exact-budget boundaries. |
|
||||
| RB-11 | [keystore-thread-safety.md](keystore-thread-safety.md) | `crates/cloud-providers/keystore.rs` | Replaced the `std::env::set_var` stub with a process-global `OnceLock<Mutex<HashMap<...>>>` keystore, keeping the API safe from any thread. Retrieval still falls back to read-only `KON_API_KEY_*` env vars for externally supplied secrets. Two regression tests cover store/retrieve and provider isolation. |
|
||||
| RB-12 | [hotkey-linux-device-filter.md](hotkey-linux-device-filter.md) | `crates/hotkey/linux.rs` | Extracted `device_supports_combo` helper; `try_attach_device` now reads the configured `HotkeyCombo` from the watch channel and checks support for that trigger key. Four regression tests land in `linux::tests`. |
|
||||
|
||||
## Dependencies
|
||||
## Remaining blocker
|
||||
|
||||
```
|
||||
RB-01 (live session race)
|
||||
└── blocked by RB-04 (run_live_session monolith refactor)
|
||||
|
||||
RB-03 (transcript-profile FK)
|
||||
└── coupled with RB-02 (migrations atomicity)
|
||||
— a v9 migration adding the FK constraint must be transactional
|
||||
```
|
||||
`RB-08` remains open pending manual runtime verification on a real macOS
|
||||
machine (`pmset -g assertions`, background live-session sanity check).
|
||||
|
||||
## How to convert to GitHub issues
|
||||
|
||||
|
||||
@@ -4,6 +4,31 @@
|
||||
**Path:** `src-tauri/src/commands/live.rs:193-338`
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md#c1--racy-single-session-guard-in-livers)
|
||||
**Labels:** release-blocker, critical, concurrency
|
||||
**Status:** RESOLVED (2026-04-22)
|
||||
|
||||
## Resolution
|
||||
|
||||
`LiveTranscriptionState` now includes a dedicated
|
||||
`tokio::sync::Mutex<()>` lifecycle gate. Both
|
||||
`start_live_transcription_session` and
|
||||
`stop_live_transcription_session` acquire that async mutex before
|
||||
touching `running`, and they keep it held across the awaited setup /
|
||||
join work that previously exposed the race windows.
|
||||
|
||||
That changes the two failing interleavings from the review:
|
||||
|
||||
- Two overlapping starts no longer race through the empty-slot check.
|
||||
The second call waits for the first to finish setup, then observes
|
||||
`running.is_some()` and returns the existing
|
||||
`"A live transcription session is already running"` error.
|
||||
- A start launched during stop can no longer sneak in after
|
||||
`running.take()` but before the previous worker has fully joined.
|
||||
It blocks on the lifecycle mutex until the join completes.
|
||||
|
||||
Regression tests in `commands::live::tests`:
|
||||
|
||||
- `concurrent_starts_allow_only_one_session_to_claim_the_slot`
|
||||
- `start_waits_for_stop_to_finish_joining_before_reusing_slot`
|
||||
|
||||
## Problem
|
||||
|
||||
@@ -26,4 +51,4 @@ Large. Will likely require the `run_live_session` monolith refactor (RB-04) to l
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **Blocked by:** RB-04 (`run_live_session` monolith refactor)
|
||||
- Landed after RB-04 (`run_live_session` refactor) made the worker lifecycle explicit enough to guard safely.
|
||||
|
||||
@@ -4,6 +4,33 @@
|
||||
**Path:** `crates/storage/src/migrations.rs:208-216`, `crates/storage/src/database.rs:61-89`, `:697-708`
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md#c4--transcript-provenance-can-reference-deleted-profiles)
|
||||
**Labels:** release-blocker, critical, data-integrity, storage
|
||||
**Status:** RESOLVED (2026-04-22)
|
||||
|
||||
## Resolution
|
||||
|
||||
Chose the strict provenance path:
|
||||
|
||||
- Migration v9 rebuilds `transcripts` with
|
||||
`profile_id REFERENCES profiles(id) ON DELETE RESTRICT`.
|
||||
- Existing orphaned transcript `profile_id` values are reconciled onto
|
||||
`DEFAULT_PROFILE_ID` during the copy into the rebuilt table.
|
||||
- Because SQLite table renames rewrite dependent references, the
|
||||
migration also rebuilds `segments`, recreates the transcript FTS
|
||||
virtual table/triggers, and repopulates FTS from the rebuilt
|
||||
transcript rows inside the same transaction.
|
||||
|
||||
Application-layer behaviour now matches the schema:
|
||||
|
||||
- `insert_transcript` rejects unknown `profile_id` values with a clear
|
||||
storage error before attempting the insert.
|
||||
- `delete_profile` returns a human-readable reassign-first error when
|
||||
transcripts still reference that profile.
|
||||
|
||||
Regression tests:
|
||||
|
||||
- `migration_v9_reconciles_orphaned_transcript_profiles_and_adds_fk`
|
||||
- `insert_transcript_rejects_unknown_profile_id`
|
||||
- `delete_profile_rejects_when_transcripts_reference_it`
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
@@ -4,6 +4,28 @@
|
||||
**Path:** `crates/cloud-providers/src/keystore.rs:6-18`
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
|
||||
**Labels:** release-blocker, major, unsafe-api, cloud
|
||||
**Status:** RESOLVED (2026-04-22)
|
||||
|
||||
## Resolution
|
||||
|
||||
Chose acceptance option 2. The environment-mutation stub is gone;
|
||||
`store_api_key` now writes into a process-global
|
||||
`OnceLock<Mutex<HashMap<String, String>>>`, so the safe signature matches
|
||||
the actual safety properties.
|
||||
|
||||
Additional details:
|
||||
|
||||
- Stored keys now live in-memory only for the life of the process.
|
||||
- `retrieve_api_key` checks the in-memory keystore first, then falls
|
||||
back to read-only `KON_API_KEY_<PROVIDER>` environment variables so
|
||||
externally injected secrets still work.
|
||||
- Module docs now describe the real tradeoff clearly: safe from any
|
||||
thread, but non-persistent until a proper OS keychain backend lands.
|
||||
|
||||
Regression tests:
|
||||
|
||||
- `stored_key_is_retrievable_without_env_mutation`
|
||||
- `providers_do_not_overlap`
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
@@ -4,6 +4,25 @@
|
||||
**Path:** `crates/llm/src/lib.rs:143-166`, `:317-321`
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
|
||||
**Labels:** release-blocker, major, llm
|
||||
**Status:** RESOLVED (2026-04-22)
|
||||
|
||||
## Resolution
|
||||
|
||||
`LlmEngine::generate` still tokenises the whole prompt up front, but it
|
||||
now runs a dedicated prompt-budget preflight before creating the llama
|
||||
context. The chosen behaviour is an early typed failure rather than
|
||||
silent truncation:
|
||||
|
||||
- If `prompt_tokens + max_tokens + 64 reserve tokens` exceeds the
|
||||
8192-token cap, generation returns
|
||||
`EngineError::PromptTooLong { prompt_tokens, max_tokens, available_prompt_tokens, context_window }`.
|
||||
- Prompts that fit exactly within the available budget still proceed and
|
||||
allocate an 8192-token context as before.
|
||||
|
||||
Regression tests:
|
||||
|
||||
- `prompt_preflight_rejects_oversized_prompt_tokens`
|
||||
- `prompt_preflight_keeps_prompts_within_budget`
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
@@ -4,6 +4,31 @@
|
||||
**Path:** `src-tauri/src/commands/live.rs:721-813`
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
|
||||
**Labels:** release-blocker, major, ipc-lifecycle
|
||||
**Status:** RESOLVED (2026-04-22)
|
||||
|
||||
## Resolution
|
||||
|
||||
`poll_inference` no longer propagates `result_channel.send(...)` with `?`.
|
||||
Instead, live-result delivery is routed through a small helper that
|
||||
tracks whether the frontend listener has already been lost:
|
||||
|
||||
- First send failure: mark the result listener as unavailable, log a
|
||||
warning, and best-effort send a `LiveStatusMessage::Warning`
|
||||
explaining that transcription will continue in the background until
|
||||
the user stops the session.
|
||||
- Subsequent chunks: skip re-sending to the dead result channel and
|
||||
keep the worker running.
|
||||
|
||||
Crucially, this path is now separate from actual transcription failure:
|
||||
inference errors still emit `LiveStatusMessage::Error` and stop the
|
||||
session, while listener-loss just stops live preview delivery.
|
||||
|
||||
Regression test:
|
||||
|
||||
- `result_listener_loss_is_warned_once_and_not_treated_as_inference_failure`
|
||||
simulates a dead result channel, confirms the first processed chunk
|
||||
downgrades to a warning, and confirms a second chunk still processes
|
||||
successfully without a second warning.
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
@@ -5,6 +5,18 @@
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md), originally deferred during A.1 #9
|
||||
**Labels:** release-blocker, major, macos, platform
|
||||
|
||||
**Current state (2026-04-23):** `objc2`/`objc2-foundation` have been
|
||||
added behind `cfg(target_os = "macos")`, and the `NSProcessInfo`
|
||||
bridge now calls `beginActivityWithOptions:reason:` / `endActivity:`
|
||||
with the retained activity handle. Isolated `cargo check` validation
|
||||
passes for both `x86_64-apple-darwin` and `aarch64-apple-darwin`.
|
||||
Remaining acceptance gap: manual runtime verification on a real macOS
|
||||
machine (`pmset -g assertions`, background live session). Diagnostic
|
||||
reports now also include a `## Power assertions` section that lists any
|
||||
currently active Kon assertion guards (`reason`, `backend`, `acquired`)
|
||||
at report time, which gives the tester an in-app breadcrumb alongside
|
||||
`pmset`.
|
||||
|
||||
## Problem
|
||||
|
||||
`begin_activity` always returns `Err` on macOS, so `PowerAssertion::begin` converts to `None` and the guard never acquires an `NSProcessInfo beginActivityWithOptions:reason:` assertion. Live recording and LLM cleanup therefore run without App Nap protection on the one platform where it matters.
|
||||
@@ -18,6 +30,21 @@ The stub was deliberate (A.1 #9 acceptance concession — untestable on Linux wi
|
||||
- `end_activity` calls `endActivity:` on the retained handle.
|
||||
- Manual-test on a real macOS box: 10-minute background live session completes without throttling; `pmset -g assertions` shows Kon's activity during capture.
|
||||
|
||||
## Manual verification checklist
|
||||
|
||||
1. Launch Kon on a real macOS machine and start a live transcription session.
|
||||
2. While capture is running, background the app for at least several minutes.
|
||||
3. In Terminal, run `pmset -g assertions` and confirm Kon appears with a
|
||||
user-initiated / no-idle-style assertion while the session is active.
|
||||
4. While the session is still running, generate a Kon diagnostic report
|
||||
and confirm the `## Power assertions` section lists an active entry
|
||||
such as `reason=kon live dictation session`, `backend=macos`,
|
||||
`acquired=true`.
|
||||
5. Stop the session and rerun `pmset -g assertions` or regenerate the
|
||||
diagnostic report to confirm the assertion disappears.
|
||||
6. Repeat once for the LLM cleanup path if desired
|
||||
(`reason=kon LLM cleanup`).
|
||||
|
||||
## Fix scope
|
||||
|
||||
Medium. Dep addition + FFI glue + manual verification. Can be done from Linux with `cargo check --target=aarch64-apple-darwin` for compile validation, but runtime behaviour needs a macOS machine.
|
||||
|
||||
@@ -4,6 +4,33 @@
|
||||
**Path:** `src-tauri/src/commands/live.rs:349-579`
|
||||
**Source:** [2026-04-22 code review](../code-review-2026-04-22.md)
|
||||
**Labels:** release-blocker, major, refactor, concurrency
|
||||
**Status:** RESOLVED (2026-04-22)
|
||||
|
||||
## Resolution
|
||||
|
||||
`run_live_session` was split around two explicit state holders:
|
||||
|
||||
- `ActiveCapture` owns the cpal stream handle, audio receiver, and
|
||||
optional runtime-error channel.
|
||||
- `LiveLoopState` owns the mutable per-session loop state: resampler,
|
||||
capture buffer, WAV writer, buffer offsets, dropped-audio accounting,
|
||||
in-flight inference task, and duplicate-history buffer.
|
||||
|
||||
The top-level worker is now `LiveSessionRuntime`, with focused methods
|
||||
for:
|
||||
|
||||
- polling in-flight inference
|
||||
- draining microphone runtime warnings
|
||||
- receiving + resampling an audio chunk
|
||||
- dropping pending-buffer overflow
|
||||
- flushing the resampler tail when stop is requested
|
||||
- dispatching inference when enough audio is buffered
|
||||
- draining the last in-flight task
|
||||
- finalising the progressive WAV writer
|
||||
|
||||
This keeps behaviour intact but removes the "everything in one mutable
|
||||
loop" shape that made concurrency review hard. The refactor also made
|
||||
RB-01 straightforward enough to land immediately afterward.
|
||||
|
||||
## Problem
|
||||
|
||||
|
||||
Reference in New Issue
Block a user