agent: lumotia-rebrand — fix Codex cross-model review blockers
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

Codex independent review found 11 blockers post-cascade. All addressed.

CRITICAL (data-loss / crash):

10. crates/core/src/paths.rs — migrate_legacy_data_dir_inner used
    fs::rename which fails with EXDEV when source + target are on
    different filesystems (encrypted-home, bind mounts, separate
    $XDG_DATA_HOME partition). Combined with the Phase 5 QC fix that
    made migration errors fatal, this would crash on first launch
    for any user whose data dir spans filesystems. Added
    rename_or_copy_tree() that falls back to copy_dir_recursive +
    remove_dir_all on CrossesDevices / errno 18 (EXDEV). Symlinks
    preserved verbatim. Same fallback applied to magnotia.db ->
    lumotia.db inside the dir.

11. Added 4 unit tests: copy_dir_recursive preserves nested
    structure, rename_or_copy_tree same-filesystem happy path,
    is_cross_device classifies CrossesDevices kind + raw errno 18.

Doc residuals (blockers 1-9):

1. crates/cloud-providers/Cargo.toml — "Wyrdnote pending rebrand"
   description.
2. crates/cloud-providers/src/provider.rs — module docs + test
   fixture Wyrdnote refs.
3. crates/transcription/src/orchestrator.rs — module docs + test
   fixture Wyrdnote refs.
4. docs/roadmap/2026-05-10-pkm-phase-tooling-shortlist.md — phase
   name + outputs/wyrdnote path refs.
5. docs/architecture-map/04-llm-formatting-mcp/llm-tests.md —
   MAGNOTIA_LLM_TEST_MODEL env var.
6. .../cloud-providers-stubs.md — MAGNOTIA_API_KEY_*.
7. docs/architecture-map/03-audio-transcription/tests-and-fixtures.md
   — MAGNOTIA_WHISPER_*.
8. docs/gpu-tuning/plan.md — MAGNOTIA_BENCH_RUN.
9. docs/superpowers/specs/2026-05-09-battery-gpu-aware-thread-tuning-
   design.md + corresponding plan — MAGNOTIA_POWER_STATE_OVERRIDE,
   MAGNOTIA_INFERENCE_THREADS.

cargo test --workspace: 343 pass / 0 fail (up from 339; +4 EXDEV
fallback tests).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-05-13 13:39:21 +01:00
parent f093d18a5e
commit 2491c7a7dd
11 changed files with 235 additions and 70 deletions

View File

@@ -48,17 +48,17 @@ All three skip silently if the env vars are unset, so `cargo test` on a fresh ma
### `whisper_rs_smoke.rs`
- Env vars: `MAGNOTIA_WHISPER_TEST_MODEL` (path to a ggml/gguf Whisper model).
- Env vars: `LUMOTIA_WHISPER_TEST_MODEL` (path to a ggml/gguf Whisper model).
- Loads the context, builds a state, calls `set_initial_prompt("Wren, CORBEL, ADHD")` (proves the API still exists), runs inference on 1 s of silence, exercises `full_n_segments` and `get_segment(i).to_str()`. Smoke test only; assertion is "did this run without panicking".
### `jfk_bench.rs`
- Env vars: `MAGNOTIA_WHISPER_TEST_MODEL`, `MAGNOTIA_WHISPER_TEST_AUDIO` (path to a 16 kHz mono 16-bit PCM WAV — assertions in the test enforce this format on the fixture, not the runtime).
- Env vars: `LUMOTIA_WHISPER_TEST_MODEL`, `LUMOTIA_WHISPER_TEST_AUDIO` (path to a 16 kHz mono 16-bit PCM WAV — assertions in the test enforce this format on the fixture, not the runtime).
- Reports cold-load time, cold-transcribe RTF, warm-transcribe RTF, peak RSS read from `/proc/{pid}/status`. Hardcodes `set_n_threads(6)` for both runs (so a sweep across thread counts is a separate test, not this one).
### `thread_sweep.rs`
- Same env vars as `jfk_bench.rs`. Adds `MAGNOTIA_POWER_STATE_OVERRIDE` (set internally per panel to `ac` or `battery`) so `inference_thread_count` returns its predicted pick for each combination of (power state × GPU offload) and the empirical RTF table can be compared against the helper's choice.
- Same env vars as `jfk_bench.rs`. Adds `LUMOTIA_POWER_STATE_OVERRIDE` (set internally per panel to `ac` or `battery`) so `inference_thread_count` returns its predicted pick for each combination of (power state × GPU offload) and the empirical RTF table can be compared against the helper's choice.
- Runs the JFK clip at `n_threads = 1, 2, 4, physical, logical` (plus `8` if logical ≥ 8), takes the min of two runs per setting, prints a four-panel table:
1. AC, CPU.
2. AC, GPU (Vulkan).
@@ -69,9 +69,9 @@ All three skip silently if the env vars are unset, so `cargo test` on a fresh ma
## Watch-outs
- **All three integration tests are quiet on missing env vars.** A green `cargo test` does NOT mean the Whisper path works; it means "either the path works or you didn't set the env vars". CI configurations must explicitly set `MAGNOTIA_WHISPER_TEST_MODEL` to exercise these.
- **All three integration tests are quiet on missing env vars.** A green `cargo test` does NOT mean the Whisper path works; it means "either the path works or you didn't set the env vars". CI configurations must explicitly set `LUMOTIA_WHISPER_TEST_MODEL` to exercise these.
- **`jfk_bench.rs` hardcodes `n_threads = 6`.** Production code uses `inference_thread_count(Workload::Whisper, gpu_offloaded)` which is power-aware. Bench numbers are not directly comparable to runtime numbers without rerunning with the helper-picked thread count.
- **`thread_sweep.rs` mutates `MAGNOTIA_POWER_STATE_OVERRIDE` via `env::set_var`.** Globally process-wide. Concurrent tests in the same `cargo test` invocation that read `MAGNOTIA_POWER_STATE_OVERRIDE` will race. The test removes the var at the end (`thread_sweep.rs:94`).
- **`thread_sweep.rs` mutates `LUMOTIA_POWER_STATE_OVERRIDE` via `env::set_var`.** Globally process-wide. Concurrent tests in the same `cargo test` invocation that read `LUMOTIA_POWER_STATE_OVERRIDE` will race. The test removes the var at the end (`thread_sweep.rs:94`).
- **`tempfile 3` is a dev-dep, not a runtime dep.** Production code never creates temp files.
- **`num_cpus = "1"` (the version, not "one CPU").** Test-only labelling; production uses the slice 5 helper.
- **In-tree HTTP fixtures use ephemeral ports (`bind 127.0.0.1:0`) and run on a tokio task.** No port-collision risk, but a paranoid sandbox that blocks raw TCP loopback would break these tests.

View File

@@ -45,7 +45,7 @@ That is the entire public surface. Anything else is an implementation detail of
`store_api_key(provider, key)` (`crates/cloud-providers/src/keystore.rs:15`):
- Acquires the global `Mutex<HashMap<String, String>>` (`api_key_store` static, `:37`).
- Inserts under the key `provider_env_key(provider)` which formats as `MAGNOTIA_API_KEY_{PROVIDER_UPPERCASED}`.
- Inserts under the key `provider_env_key(provider)` which formats as `LUMOTIA_API_KEY_{PROVIDER_UPPERCASED}`.
- Returns nothing — last-write-wins.
`retrieve_api_key(provider)` (`crates/cloud-providers/src/keystore.rs:27`):
@@ -54,7 +54,7 @@ That is the entire public surface. Anything else is an implementation detail of
- Falls back to `std::env::var(env_key)` if not present in memory.
- Returns `Option<String>`.
The fallback is the why behind the `MAGNOTIA_API_KEY_<PROVIDER>` naming convention: an operator can inject a key via the environment without going through the in-memory store, which is useful for headless / CI runs.
The fallback is the why behind the `LUMOTIA_API_KEY_<PROVIDER>` naming convention: an operator can inject a key via the environment without going through the in-memory store, which is useful for headless / CI runs.
### Documented TODO
@@ -90,7 +90,7 @@ Today:
```
caller (currently no in-tree caller)
→ store_api_key(provider, key)
→ api_key_store().lock().insert("MAGNOTIA_API_KEY_{PROVIDER}", key)
→ api_key_store().lock().insert("LUMOTIA_API_KEY_{PROVIDER}", key)
→ retrieve_api_key(provider)
→ check in-memory map
→ fall back to std::env::var
@@ -113,7 +113,7 @@ The formatting pipeline does not need to change to consume cloud-transcribed seg
- **Empty crate is intentional.** Removing it would be premature; the workspace shape and the BYOK plan are both implied by its presence. But anyone doing slice analysis ("what does this crate do?") needs to know it does almost nothing today.
- **API keys vanish on restart.** The TODO is explicit. Until `keyring` integration lands, every cloud-provider feature using these helpers will need to re-prompt the user on every startup or break for headless deployments. Acceptable for a stub; not acceptable for a shipped feature.
- **Env-var fallback is `MAGNOTIA_API_KEY_<PROVIDER>`.** Provider names are uppercased in the env-key construction. A provider name with hyphens or underscores will produce a slightly weird-looking env var; not broken, but worth knowing. `provider = "open-ai"` becomes `MAGNOTIA_API_KEY_OPEN-AI`.
- **Env-var fallback is `LUMOTIA_API_KEY_<PROVIDER>`.** Provider names are uppercased in the env-key construction. A provider name with hyphens or underscores will produce a slightly weird-looking env var; not broken, but worth knowing. `provider = "open-ai"` becomes `LUMOTIA_API_KEY_OPEN-AI`.
- **No threading concerns beyond the mutex.** `Mutex<HashMap>` is fine here because keys are written rarely and read on the path of a network call that dwarfs any contention. The previous note in the doc-comment about "undefined behaviour of mutating process environment variables from arbitrary threads" refers to a discarded design that used `std::env::set_var` — that approach is unsound under the modern Rust memory model. The current in-memory map is the safe replacement.
- **`lumotia-core` dependency is declared but not currently used in this file.** It is reserved for when the providers themselves arrive (they will likely consume `Segment` and tuning helpers).
- **Security of the in-memory map is process-lifetime only.** A core dump or a memory-inspection attack reveals the keys. The `keyring`-backed replacement will inherit OS-level protections; until then the threat model is "user trusts their own machine".

View File

@@ -9,7 +9,7 @@ last_verified: 2026/05/09
> **Where you are:** [Architecture map](../README.md) → [LLM, Formatting, MCP](README.md) → Tests
**Plain English summary.** Two integration smoke tests live under `crates/llm/tests/`. Both gate on the `MAGNOTIA_LLM_TEST_MODEL` environment variable and skip silently when it is not set. They exist to verify that real model loading and inference works end-to-end, but never run in default `cargo test` runs because model load is heavy.
**Plain English summary.** Two integration smoke tests live under `crates/llm/tests/`. Both gate on the `LUMOTIA_LLM_TEST_MODEL` environment variable and skip silently when it is not set. They exist to verify that real model loading and inference works end-to-end, but never run in default `cargo test` runs because model load is heavy.
## At a glance
@@ -44,7 +44,7 @@ Verifies the Phase 9 `extract_content_tags` surface against a real loaded model.
Path-and-line summary:
- Env-var gate at `crates/llm/tests/content_tags_smoke.rs:17-23`. Same `MAGNOTIA_LLM_TEST_MODEL` as `smoke.rs`.
- Env-var gate at `crates/llm/tests/content_tags_smoke.rs:17-23`. Same `LUMOTIA_LLM_TEST_MODEL` as `smoke.rs`.
- Loads the 2B tier (the smoke tests deliberately use the smallest tier so they run quickly even on modest hardware).
- Realistic transcript: a multi-sentence dictation about a grant application and a meeting.
- Calls `engine.extract_content_tags(transcript)` and asserts:
@@ -57,7 +57,7 @@ The character-class assertion mirrors `CONTENT_TAGS_GRAMMAR`'s `topic-char ::= [
### Run command (from both files' header comments)
```bash
MAGNOTIA_LLM_TEST_MODEL=/path/to/model.gguf cargo test -p lumotia-llm \
LUMOTIA_LLM_TEST_MODEL=/path/to/model.gguf cargo test -p lumotia-llm \
--test content_tags_smoke -- --nocapture
```
@@ -84,7 +84,7 @@ Default `cargo test -p lumotia-llm` runs the unit tests (no model load) plus ski
## Data flow (for the smoke tests)
```
env MAGNOTIA_LLM_TEST_MODEL
env LUMOTIA_LLM_TEST_MODEL
→ if unset: print message, return (no failure)
→ else: PathBuf
→ LlmEngine::new()
@@ -99,7 +99,7 @@ env MAGNOTIA_LLM_TEST_MODEL
- **Smoke tests need a 2B model file.** The 2B Q4_K_M GGUF is ~1.28 GB; downloading it once is a manual prerequisite. CI is not currently configured to fetch it.
- **Smoke tests load the 2B tier specifically.** The smoke test was written against the smallest tier so it runs in a few seconds on a modest machine. Running on the 27B tier through this path would take minutes per test and would saturate VRAM.
- **No content_tags smoke for the empty-transcript error path.** The test only exercises the happy path. The error path is covered by `extract_content_tags`'s logic and would only surface here if a regression broke the typed deserialise.
- **`MAGNOTIA_LLM_TEST_MODEL` is the only env-gate.** No second gate for "I have a Vulkan-capable GPU available". `use_gpu: true` is hard-coded; on a CPU-only machine the test will still pass but run slower and produce a llama.cpp warning.
- **`LUMOTIA_LLM_TEST_MODEL` is the only env-gate.** No second gate for "I have a Vulkan-capable GPU available". `use_gpu: true` is hard-coded; on a CPU-only machine the test will still pass but run slower and produce a llama.cpp warning.
## See also