diff --git a/Cargo.lock b/Cargo.lock index dada4040e..5d90e7c71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2225,7 +2225,6 @@ dependencies = [ "rand_distr", "serde", "serde_json", - "serial_test", "tempfile", "thiserror 2.0.18", "zip 2.4.2", diff --git a/crates/larql-kv/Cargo.toml b/crates/larql-kv/Cargo.toml index 110b9798d..551d01880 100644 --- a/crates/larql-kv/Cargo.toml +++ b/crates/larql-kv/Cargo.toml @@ -50,10 +50,6 @@ gpu = ["dep:larql-compute-metal", "larql-inference/gpu"] [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } tempfile = "3" -# Serialise tests that read process-global env vars (LARQL_W10_DISABLE -# dispatch cascade, LARQL_MARKOV_* compute-path diagnostics) so they -# don't race other tests reading the same variables in parallel. -serial_test = "3.2" [[bench]] name = "engine_decode" diff --git a/crates/larql-kv/ROADMAP.md b/crates/larql-kv/ROADMAP.md index 8026181aa..541f58101 100644 --- a/crates/larql-kv/ROADMAP.md +++ b/crates/larql-kv/ROADMAP.md @@ -47,15 +47,15 @@ entirely (W10 — engine-side state lives on GPU until window-close). - in-crate `benches/engine_decode.rs` (criterion: dispatch helpers + Standard parity) - Coverage policy: 90 % line coverage per source file (see `coverage-policy.json`); CI gate at `make larql-kv-coverage-policy`. - Workspace `larql-kv` lib total: **95.55% lines, 95.40% regions, 94.49% - functions** (2026-05-17 evening, up from 92.12% earlier the same day). - **All 43 files now ≥90% lines; debt baselines cleared from policy - file.** The post-Phase-2 push lifted `accuracy_suite/measurement.rs` - (77→99%), `accuracy_suite/runner.rs` (75→94%), `vindex_compare.rs` - (65→97%), and `engines/apollo/store.rs` (89→92%) by adding - formatter + driver tests against the synthetic fixtures; the - Phase-2 engines (`unlimited_context/engine.rs`, `turbo_quant/engine.rs`, - `apollo/engine.rs`) all land ≥92% on the new `*_via_executor` methods. + Workspace `larql-kv` lib total: **95.62% lines, 95.43% regions, 95.50% + functions** (2026-05-24 evening, post coverage-debt clearance). + **All 61 files at ≥90% lines; debt baselines cleared from policy + file.** The 2026-05-24 push lifted the five `engines/*/dispatch.rs` + files (range 7.95–80.68% → 93.57–97.85%) and + `engines/markov_residual/compute.rs` (86.85→95.30%). See "Closed + (recent)" entry for the thread-local-override pattern that makes + the env-gated paths in `compute.rs` and the W10 mask cascade in + the dispatch files testable without process-env mutation. ## Architectural cuts (2026-05-17) @@ -125,95 +125,66 @@ Metal calls `larql_inference::vindex::fused_prefill`. ## Coverage debt -**Status (2026-05-24):** six files below the 90% per-file floor after -the Phase-2 dispatch additions and the env-var-gated diagnostic -paths in `compute.rs` drifted past the 2026-05-17 cleared state. The -post-Item-2 cross-product work added five new files (`runner/*.rs`) -all above 90%; the regression is in older engine internals that -weren't covered when the dispatch paths landed in May. - -| File | Coverage | Lift path | -|---|---:|---| -| `engines/markov_residual/compute.rs` | 86.85% | env-var-gated paths (`LARQL_MARKOV_WALK_KV_DIAG`, `_FORCE_F32`, `_TOPK`) — needs `serial_test` crate or config-injection refactor | -| `engines/unlimited_context/dispatch.rs` | 59.09% | needs mock `EngineBackend` infrastructure | -| `engines/markov_residual/dispatch.rs` | 77.51% | needs mock `EngineBackend` infrastructure | -| `engines/markov_residual_codec/dispatch.rs` | 80.68% | needs mock `EngineBackend` infrastructure | -| `engines/turbo_quant/dispatch.rs` | 9.35% | needs mock `EngineBackend` infrastructure | -| `engines/boundary_per_layer/dispatch.rs` | 7.95% | needs mock `EngineBackend` infrastructure | - -Progress 2026-05-24 (session-level): lifted five files above the -floor (cold_tier 88→100%, executor 85→90.6%, walk 84→95%, engine -83→90%, store 86→99.6%) and improved compute.rs from 81→86.85% -without env-var test infrastructure. 37 new tests across the five -fixed files; clippy clean; zero regressions. - -### What's left - -**Sub-project A — mock `EngineBackend` infrastructure (5 files).** All -five remaining dispatch files call -`backend.coarse_prefill_with_state` and -`backend.coarse_decode_step_with_state_masked` — the GPU-dispatch -surface that only `MetalBackend` implements meaningfully today. The -existing 59–80% coverage on three of them comes from -`tests/dispatch_parity.rs` (integration tests against real engines); -the 0-10% files (`boundary_per_layer/dispatch.rs`, -`turbo_quant/dispatch.rs`) aren't even reached by those. - -The lift requires a shared test-only `EngineBackend` impl that -returns synthetic `PerLayerDecodeState` payloads conformant with the -trait. Once built, each dispatch file gets ~5 unit tests exercising -prefill happy-path, prefill no-state-dump fallback, decode happy-path, -decode `StateDumpMask::HOnly` / `None` cascades. Open question: where -the mock lives — `larql-inference::test_utils` (next to -`make_test_weights`) or a new `larql-kv/src/test_utils.rs` that -imports the trait. The former is more reusable across crates; the -latter is local to where the mock is consumed. - -Estimated scope: ~1 hour for the mock infra + 30 min per dispatch -file = 3-4 hours total. - -**Sub-project B — `compute.rs` env-var path coverage (1 file).** The -remaining ~7% gap is the diagnostic + force-f32 code paths gated on -`LARQL_MARKOV_WALK_KV_DIAG`, `LARQL_MARKOV_KV_FORCE_F32`, -`LARQL_MARKOV_WALK_KV_TOPK`, `LARQL_MARKOV_WALK_KV_SELECT_AT`. These -read process env vars at decode time — testing them in parallel -requires `serial_test` (or similar) to serialise `env::set_var` calls -without racing other tests. Alternative: refactor the helpers to -take an explicit config struct argument; the env-var read becomes a -single thin caller and the tests can pass values directly without -touching the global env. - -Estimated scope: 45 min for `serial_test` integration, 1-2 hours for -the config-injection refactor. - -**Acceptance criterion.** `make larql-kv-coverage-policy` passes -against a *freshly regenerated* `coverage/larql-kv/summary.json` -(via `make larql-kv-coverage-summary`, not the cached JSON). 2026-05-24 -note: the gate was previously passing against a stale JSON that -predated the dispatch additions — fresh regeneration surfaced the -debt. Future commits to engine internals should run -`make larql-kv-coverage-summary` locally, not just -`make larql-kv-coverage-policy`. - -### Open design questions - -1. **Should the mock `EngineBackend` live in `larql-inference::test_utils` - or in `larql-kv`?** Reusable-across-crates favours the former; - local-to-consumers favours the latter. -2. **`serial_test` crate vs env-var-injection refactor for `compute.rs`?** - The crate is a small dependency add; the refactor is a more - invasive change but yields more testable code long-term. -3. **Acceptance for the GPU-only branches.** Some lines in the - dispatch files only fire on a real Metal backend (e.g. - `StateDumpMask::Full` paths that exercise the cached-decode - kernel). Mock-backend tests can verify the *plumbing* but not - the *kernel* — should those lines be excluded via - `#[cfg(not(test_coverage))]` or similar, or accepted as - coverable-only-via-integration-tests? - -These are blocking on the implementation slice landing, not the -coverage-debt resolution itself; the design choices fall during the -sub-project work, not before. +**Status (2026-05-24 — CLOSED.)** All six files below the 90% per-file +floor have been lifted; `make larql-kv-coverage-policy` passes +against fresh `summary.json` regeneration. Workspace total 95.62% +lines, 61/61 files at ≥90%, 0 debt baselines in +`coverage-policy.json`. + +| File | Pre | Post | +|---|---:|---:| +| `engines/markov_residual/compute.rs` | 86.85% | **95.30%** | +| `engines/unlimited_context/dispatch.rs` | 59.09% | **97.24%** | +| `engines/markov_residual/dispatch.rs` | 77.51% | **96.78%** | +| `engines/markov_residual_codec/dispatch.rs` | 80.68% | **97.72%** | +| `engines/turbo_quant/dispatch.rs` | 9.35% | **97.85%** | +| `engines/boundary_per_layer/dispatch.rs` | 7.95% | **93.57%** | + +**Implementation summary.** No new shared mock infrastructure was +needed: `CpuBackend` (via `cpu_engine_backend()`) already implements +`coarse_*_with_state` for the synthetic Q4K fixture +(`make_test_q4k_weights` + `make_test_q4k_vindex`), which drives +every dispatch happy-path through real per-layer state capture. +~50 new `#[cfg(test)] mod tests` cases added inline per dispatch +file plus ~10 env-var-gated cases in `compute.rs`. Zero regressions; +`make larql-kv-ci` passes. + +**Env-var-gated paths — thread-local override pattern.** The +`LARQL_MARKOV_*` (compute.rs walk-KV diagnostics) and +`LARQL_W10_DISABLE` (dispatch mask cascade) helpers were +near-impossible to test safely under `cargo test --jobs N`: setting +process-global env from one test races every other parallel test +that consults the same var (caught a real flake in +`prefill_with_overflow_creates_encoded_cold_tier`). Resolution: each +env helper now consults a per-thread `RefCell` override map +*before* falling back to `std::env::var`. Tests inject values into +the thread-local; production reads env unchanged. No `serial_test` +crate needed, no `#[serial]` annotations, no env mutation. The +helpers: + +- `compute.rs::read_markov_env(key)` + `set_markov_env_override(...)` / + `clear_markov_env_overrides()` (test-only). +- `engines/mod.rs::w10_enabled()` + `set_w10_disabled_override(...)` + (test-only). + +**Open design questions — resolved by the work above.** + +1. *Mock `EngineBackend` location* — moot. `CpuBackend` is the mock; + nothing new was added. +2. *`serial_test` vs config-injection refactor* — chose neither. + Thread-local override (per-test isolation without process + mutation) is the third option and the right one. +3. *GPU-only dispatch branches* — non-issue at current coverage. + Every dispatch file lands at ≥93% via the CPU happy path; the + Metal-only `StateDumpMask::Full` blit branches are exercised + indirectly by `CpuBackend`'s in-process implementation. No + `cfg`-gating needed. + +**Lesson for future env-gated production code:** add the +thread-local override at the same time as the `std::env::var` read, +not as a follow-on. Saves the future test-author from picking +between flaky parallel tests, `serial_test` ceremony, or a +config-injection refactor. ## Open work @@ -809,6 +780,38 @@ were implementation). ## Closed (recent) +- **2026-05-24 — Coverage debt CLEARED.** All six files below the + 90% per-file floor lifted; `make larql-kv-coverage-policy` passes + against fresh `summary.json` regeneration. Workspace total 95.62% + lines, 61/61 files at ≥90%, 0 debt baselines remaining. + + Files lifted (pre → post): `turbo_quant/dispatch` 9.35→97.85%, + `boundary_per_layer/dispatch` 7.95→93.57%, `unlimited_context/dispatch` + 59.09→97.24%, `markov_residual/dispatch` 77.51→96.78%, + `markov_residual_codec/dispatch` 80.68→97.72%, + `markov_residual/compute` 86.85→95.30%. + + Approach inverted both pre-baked design assumptions: + - **No new shared mock `EngineBackend`** — `CpuBackend` (via + `cpu_engine_backend()`) already implements `coarse_*_with_state` + when driven against the synthetic Q4K fixture + (`make_test_q4k_weights` + `make_test_q4k_vindex`), so every + dispatch happy-path tested end-to-end without new infrastructure. + - **No `serial_test` crate** — env-gated paths + (`LARQL_MARKOV_WALK_KV_*`, `LARQL_W10_DISABLE`) instead gained + a per-thread `RefCell` override that production helpers consult + *before* `std::env::var`. Tests inject without touching the + process env; no race with other parallel tests. New helpers: + `compute.rs::set_markov_env_override(...)`, + `engines/mod.rs::set_w10_disabled_override(...)` (both + `#[cfg(test)]` only). + + Test deltas: larql-kv lib 663 → 712 (+49). Zero regressions + (5/5 successive `cargo test -p larql-kv --lib` runs green after + the thread-local override fix; pre-fix the env-var-setting tests + produced flaky `cold_kv.is_some()` failures in unrelated codec + tests via process-env race). `make larql-kv-ci` passes end-to-end. + - **2026-05-24 — Accuracy harness honesty + FFN policy cross-product LANDED.** Multi-PR arc that turns the accuracy suite from "silent drop on engine miss" into a discriminating cross-product harness: diff --git a/crates/larql-kv/src/engines/boundary_per_layer/dispatch.rs b/crates/larql-kv/src/engines/boundary_per_layer/dispatch.rs index b91f822b2..1c1550713 100644 --- a/crates/larql-kv/src/engines/boundary_per_layer/dispatch.rs +++ b/crates/larql-kv/src/engines/boundary_per_layer/dispatch.rs @@ -236,7 +236,6 @@ mod tests { use larql_inference::test_utils::{ make_test_q4k_vindex, make_test_q4k_weights, Q4K_TEST_NUM_LAYERS, }; - use serial_test::serial; use super::*; use crate::engines::boundary_per_layer::policy::BoundaryLayerPolicy; @@ -245,17 +244,16 @@ mod tests { BoundaryLayerPolicy::bf16_uniform("test", Q4K_TEST_NUM_LAYERS) } - /// Reset `LARQL_W10_DISABLE` to a known state. Every test in this - /// module exercises code paths gated on `w10_env_on()` (which reads - /// this var), so the tests are `#[serial]` and start clean. - fn clear_w10_env() { - std::env::remove_var("LARQL_W10_DISABLE"); + /// Clear the per-thread W10 cascade override so the engine reads + /// the (unset, default-on) env. Tests call this at the start to + /// neutralise overrides leaked by earlier tests on the same thread. + fn clear_w10_override() { + crate::engines::set_w10_disabled_override(None); } #[test] - #[serial] fn try_prefill_via_dispatch_returns_none_when_index_lacks_direct_matvec() { - clear_w10_env(); + clear_w10_override(); let weights = make_test_q4k_weights(); let empty_index = larql_vindex::VectorIndex::new( vec![None; weights.num_layers], @@ -277,9 +275,8 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowed_populates_store_under_w10_honly() { - clear_w10_env(); + clear_w10_override(); let mut weights = make_test_q4k_weights(); let index = make_test_q4k_vindex(&weights); let backend = cpu_engine_backend(); @@ -302,9 +299,8 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowless_drops_stored_under_w10_none_mask() { - clear_w10_env(); + clear_w10_override(); let mut weights = make_test_q4k_weights(); let index = make_test_q4k_vindex(&weights); let backend = cpu_engine_backend(); @@ -326,9 +322,8 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_appends_h_in_under_honly() { - clear_w10_env(); + clear_w10_override(); let mut weights = make_test_q4k_weights(); let index = make_test_q4k_vindex(&weights); let backend = cpu_engine_backend(); @@ -359,9 +354,8 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_windowless_takes_none_mask_path() { - clear_w10_env(); + clear_w10_override(); let mut weights = make_test_q4k_weights(); let index = make_test_q4k_vindex(&weights); let backend = cpu_engine_backend(); @@ -392,9 +386,8 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_overflow_extends_cold_tier() { - clear_w10_env(); + clear_w10_override(); let mut weights = make_test_q4k_weights(); let index = make_test_q4k_vindex(&weights); let backend = cpu_engine_backend(); diff --git a/crates/larql-kv/src/engines/markov_residual/compute.rs b/crates/larql-kv/src/engines/markov_residual/compute.rs index 44eac8b8e..cefcca09f 100644 --- a/crates/larql-kv/src/engines/markov_residual/compute.rs +++ b/crates/larql-kv/src/engines/markov_residual/compute.rs @@ -34,6 +34,46 @@ struct WalkKvSelection { thread_local! { static WALK_KV_SELECTION: RefCell> = const { RefCell::new(None) }; + /// Per-thread override for `LARQL_MARKOV_*` env vars consulted by + /// the walk-KV helpers below. Tests set entries here to exercise + /// the env-gated branches without mutating the process-global env + /// (which would race other parallel tests in the same crate that + /// also call `recompute_kv`). Production code is unaffected — when + /// the thread-local is empty the helpers fall through to + /// `std::env::var`. + static MARKOV_ENV_OVERRIDE: RefCell>> = + RefCell::new(std::collections::HashMap::new()); +} + +/// Read an env var subject to thread-local overrides (test-only escape +/// hatch — see `MARKOV_ENV_OVERRIDE`). An override of `Some(value)` +/// behaves like the env var being set to that value; `None` behaves +/// like the var being unset. With no override the helper delegates to +/// the real process env, so production callers see no change. +fn read_markov_env(key: &'static str) -> Option { + let overridden = MARKOV_ENV_OVERRIDE.with(|o| { + o.borrow() + .get(key) + .map(|v| (true, v.clone())) + .unwrap_or((false, None)) + }); + if overridden.0 { + overridden.1 + } else { + std::env::var(key).ok() + } +} + +#[cfg(test)] +fn set_markov_env_override(key: &'static str, value: Option<&str>) { + MARKOV_ENV_OVERRIDE.with(|o| { + o.borrow_mut().insert(key, value.map(|s| s.to_string())); + }); +} + +#[cfg(test)] +fn clear_markov_env_overrides() { + MARKOV_ENV_OVERRIDE.with(|o| o.borrow_mut().clear()); } pub struct RsPrefillResult { @@ -503,7 +543,7 @@ fn markov_walk_kv_top_k(layer: usize, kv_dim: usize) -> Option { return None; } } - if let Ok(spec) = std::env::var("LARQL_MARKOV_WALK_KV_LAYERS") { + if let Some(spec) = read_markov_env("LARQL_MARKOV_WALK_KV_LAYERS") { if !layer_in_spec(&spec, layer) { return None; } @@ -512,7 +552,7 @@ fn markov_walk_kv_top_k(layer: usize, kv_dim: usize) -> Option { } fn markov_walk_kv_requested_top_k(kv_dim: usize) -> Option { - let raw = std::env::var("LARQL_MARKOV_WALK_KV_TOPK").ok()?; + let raw = read_markov_env("LARQL_MARKOV_WALK_KV_TOPK")?; let top_k = raw.trim().parse::().ok()?; if top_k == 0 { return None; @@ -521,22 +561,19 @@ fn markov_walk_kv_requested_top_k(kv_dim: usize) -> Option { } fn markov_walk_kv_select_at() -> Option { - std::env::var("LARQL_MARKOV_WALK_KV_SELECT_AT") - .ok()? + read_markov_env("LARQL_MARKOV_WALK_KV_SELECT_AT")? .trim() .parse() .ok() } fn markov_walk_kv_diag_enabled() -> bool { - std::env::var("LARQL_MARKOV_WALK_KV_DIAG") - .ok() + read_markov_env("LARQL_MARKOV_WALK_KV_DIAG") .is_some_and(|v| matches!(v.trim(), "1" | "true" | "TRUE" | "yes" | "on")) } fn markov_kv_force_f32_projection() -> bool { - std::env::var("LARQL_MARKOV_KV_FORCE_F32") - .ok() + read_markov_env("LARQL_MARKOV_KV_FORCE_F32") .is_some_and(|v| matches!(v.trim(), "1" | "true" | "TRUE" | "yes" | "on")) } @@ -544,9 +581,7 @@ fn markov_walk_kv_diag_layer(layer: usize) -> bool { // `is_none_or` is MSRV 1.82; project pins MSRV 1.80. Equivalent // semantics: env-var absent → true (diag applies to all layers), // env-var present → check the comma-list. - std::env::var("LARQL_MARKOV_WALK_KV_LAYERS") - .ok() - .map_or(true, |spec| layer_in_spec(&spec, layer)) + read_markov_env("LARQL_MARKOV_WALK_KV_LAYERS").map_or(true, |spec| layer_in_spec(&spec, layer)) } fn layer_in_spec(spec: &str, layer: usize) -> bool { @@ -1171,178 +1206,145 @@ mod tests { // ── Env-var-gated walk-KV paths ─────────────────────────────────────────── // // These tests cover the `LARQL_MARKOV_WALK_KV_*` / - // `LARQL_MARKOV_KV_FORCE_F32` paths in `recompute_kv` + - // `markov_walk_kv_*` helpers. The env vars are read at decode - // time via `std::env::var`, so each test is `#[serial]` (the - // `serial_test` crate) and resets the relevant vars at the start - // to prevent prior-test leakage. - - fn clear_markov_walk_env() { - for var in [ - "LARQL_MARKOV_WALK_KV_TOPK", - "LARQL_MARKOV_WALK_KV_LAYERS", - "LARQL_MARKOV_WALK_KV_SELECT_AT", - "LARQL_MARKOV_WALK_KV_DIAG", - "LARQL_MARKOV_KV_FORCE_F32", - ] { - std::env::remove_var(var); - } - } + // `LARQL_MARKOV_KV_FORCE_F32` paths in `recompute_kv` and the + // `markov_walk_kv_*` helpers. Production reads via + // `read_markov_env`, which consults the per-thread + // `MARKOV_ENV_OVERRIDE` map *before* `std::env::var`. Tests inject + // values through `set_markov_env_override` — no process-global env + // mutation, no `#[serial]` needed, no race with other parallel + // tests that also call `recompute_kv`. #[test] - #[serial_test::serial] fn markov_walk_kv_requested_top_k_parses_clamps_and_rejects_zero() { - clear_markov_walk_env(); - // Not set → None. + clear_markov_env_overrides(); assert_eq!(markov_walk_kv_requested_top_k(32), None); - // Set → parsed; clamp to kv_dim. - std::env::set_var("LARQL_MARKOV_WALK_KV_TOPK", "8"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_TOPK", Some("8")); assert_eq!(markov_walk_kv_requested_top_k(32), Some(8)); assert_eq!( markov_walk_kv_requested_top_k(4), Some(4), "clamp to kv_dim" ); - // Zero is rejected. - std::env::set_var("LARQL_MARKOV_WALK_KV_TOPK", "0"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_TOPK", Some("0")); assert_eq!(markov_walk_kv_requested_top_k(32), None); - // Garbage rejects. - std::env::set_var("LARQL_MARKOV_WALK_KV_TOPK", "abc"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_TOPK", Some("abc")); assert_eq!(markov_walk_kv_requested_top_k(32), None); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn markov_walk_kv_select_at_parses_layer_index() { - clear_markov_walk_env(); + clear_markov_env_overrides(); assert_eq!(markov_walk_kv_select_at(), None); - std::env::set_var("LARQL_MARKOV_WALK_KV_SELECT_AT", "7"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_SELECT_AT", Some("7")); assert_eq!(markov_walk_kv_select_at(), Some(7)); - std::env::set_var("LARQL_MARKOV_WALK_KV_SELECT_AT", "bad"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_SELECT_AT", Some("bad")); assert_eq!(markov_walk_kv_select_at(), None); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn markov_walk_kv_diag_enabled_accepts_truthy_strings() { - clear_markov_walk_env(); + clear_markov_env_overrides(); assert!(!markov_walk_kv_diag_enabled()); for val in ["1", "true", "TRUE", "yes", "on"] { - std::env::set_var("LARQL_MARKOV_WALK_KV_DIAG", val); + set_markov_env_override("LARQL_MARKOV_WALK_KV_DIAG", Some(val)); assert!(markov_walk_kv_diag_enabled(), "should accept {val}"); } for val in ["0", "false", "no"] { - std::env::set_var("LARQL_MARKOV_WALK_KV_DIAG", val); + set_markov_env_override("LARQL_MARKOV_WALK_KV_DIAG", Some(val)); assert!(!markov_walk_kv_diag_enabled(), "should reject {val}"); } - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn markov_kv_force_f32_projection_reads_env() { - clear_markov_walk_env(); + clear_markov_env_overrides(); assert!(!markov_kv_force_f32_projection()); - std::env::set_var("LARQL_MARKOV_KV_FORCE_F32", "1"); + set_markov_env_override("LARQL_MARKOV_KV_FORCE_F32", Some("1")); assert!(markov_kv_force_f32_projection()); - std::env::set_var("LARQL_MARKOV_KV_FORCE_F32", "no"); + set_markov_env_override("LARQL_MARKOV_KV_FORCE_F32", Some("no")); assert!(!markov_kv_force_f32_projection()); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn markov_walk_kv_diag_layer_respects_layers_spec() { - clear_markov_walk_env(); - // Absent → every layer counts. + clear_markov_env_overrides(); assert!(markov_walk_kv_diag_layer(0)); assert!(markov_walk_kv_diag_layer(99)); - // Restricted to range → outside layers excluded. - std::env::set_var("LARQL_MARKOV_WALK_KV_LAYERS", "3-5"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_LAYERS", Some("3-5")); assert!(markov_walk_kv_diag_layer(4)); assert!(!markov_walk_kv_diag_layer(0)); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn markov_walk_kv_top_k_honours_layers_and_select_at_gates() { - clear_markov_walk_env(); - // Not requested → None. + clear_markov_env_overrides(); assert_eq!(markov_walk_kv_top_k(0, 32), None); - // Requested but layer not in spec → None. - std::env::set_var("LARQL_MARKOV_WALK_KV_TOPK", "4"); - std::env::set_var("LARQL_MARKOV_WALK_KV_LAYERS", "5-7"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_TOPK", Some("4")); + set_markov_env_override("LARQL_MARKOV_WALK_KV_LAYERS", Some("5-7")); assert_eq!(markov_walk_kv_top_k(0, 32), None); assert_eq!(markov_walk_kv_top_k(6, 32), Some(4)); - // SELECT_AT == layer → None (selector layer keeps the dense path). - std::env::remove_var("LARQL_MARKOV_WALK_KV_LAYERS"); - std::env::set_var("LARQL_MARKOV_WALK_KV_SELECT_AT", "6"); + set_markov_env_override("LARQL_MARKOV_WALK_KV_LAYERS", None); + set_markov_env_override("LARQL_MARKOV_WALK_KV_SELECT_AT", Some("6")); assert_eq!(markov_walk_kv_top_k(6, 32), None); assert_eq!(markov_walk_kv_top_k(7, 32), Some(4)); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn recompute_kv_force_f32_disables_q4k_path() { - clear_markov_walk_env(); - std::env::set_var("LARQL_MARKOV_KV_FORCE_F32", "1"); + clear_markov_env_overrides(); + set_markov_env_override("LARQL_MARKOV_KV_FORCE_F32", Some("1")); let weights = make_test_weights(); let h = Array2::from_elem((2, weights.hidden_size), 0.5f32); let (k, v) = recompute_kv(&weights, &h, 0, 0, &CpuBackend, None).unwrap(); let kv_dim = weights.num_kv_heads * weights.head_dim; assert_eq!(k.shape(), &[2, kv_dim]); assert_eq!(v.shape(), &[2, kv_dim]); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn recompute_kv_topk_routes_through_walk_projection() { - clear_markov_walk_env(); - // top_k forces the walk-KV f32 fallback regardless of backend. - std::env::set_var("LARQL_MARKOV_WALK_KV_TOPK", "2"); + clear_markov_env_overrides(); + set_markov_env_override("LARQL_MARKOV_WALK_KV_TOPK", Some("2")); let weights = make_test_weights(); let h = Array2::from_elem((2, weights.hidden_size), 0.25f32); let result = recompute_kv(&weights, &h, 0, 0, &CpuBackend, None); assert!(result.is_some()); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn recompute_kv_select_at_uses_cached_indices_on_later_layers() { - clear_markov_walk_env(); - std::env::set_var("LARQL_MARKOV_WALK_KV_TOPK", "2"); - std::env::set_var("LARQL_MARKOV_WALK_KV_SELECT_AT", "0"); + clear_markov_env_overrides(); + set_markov_env_override("LARQL_MARKOV_WALK_KV_TOPK", Some("2")); + set_markov_env_override("LARQL_MARKOV_WALK_KV_SELECT_AT", Some("0")); let weights = make_test_weights(); let h = Array2::from_elem((2, weights.hidden_size), 0.25f32); - // Layer 0: should_cache_selection fires, populates the - // thread-local; layer 1: walk_project_cached_topk reads it. + // Layer 0: should_cache_selection fires, populates + // WALK_KV_SELECTION; layer 1: walk_project_cached_topk reads it. let _ = recompute_kv(&weights, &h, 0, 0, &CpuBackend, None); if weights.num_layers >= 2 { let result = recompute_kv(&weights, &h, 1, 0, &CpuBackend, None); assert!(result.is_some()); } - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] - #[serial_test::serial] fn recompute_kv_diag_fires_when_enabled() { - clear_markov_walk_env(); - std::env::set_var("LARQL_MARKOV_WALK_KV_DIAG", "1"); + clear_markov_env_overrides(); + set_markov_env_override("LARQL_MARKOV_WALK_KV_DIAG", Some("1")); let weights = make_test_weights(); let h = Array2::from_elem((1, weights.hidden_size), 0.5f32); - // Diagnostic path runs `print_walk_kv_diag` — body is - // observability output; we just verify no panic and a valid - // result. let result = recompute_kv(&weights, &h, 0, 0, &CpuBackend, None); assert!(result.is_some()); - clear_markov_walk_env(); + clear_markov_env_overrides(); } #[test] diff --git a/crates/larql-kv/src/engines/markov_residual/dispatch.rs b/crates/larql-kv/src/engines/markov_residual/dispatch.rs index a8d97db84..ab84e8caf 100644 --- a/crates/larql-kv/src/engines/markov_residual/dispatch.rs +++ b/crates/larql-kv/src/engines/markov_residual/dispatch.rs @@ -268,22 +268,17 @@ impl MarkovResidualEngine { #[cfg(test)] mod tests { //! Coverage for the W1-GPU dispatch path. Drives `CpuBackend` via - //! the synthetic Q4K fixture; W10 mask cascade is on by default, - //! so windowed configurations hit the `HOnly` path and windowless - //! configurations hit `None`. The `Full` path is exercised with - //! `LARQL_W10_DISABLE=1`. - //! - //! Every test is `#[serial]` because `w10_enabled()` reads the - //! process-global `LARQL_W10_DISABLE` env var — running in parallel - //! would race the env state. Each test resets the env at start via - //! [`set_w10_disable`] so prior runs can't leak. + //! the synthetic Q4K fixture. W10 mask cascade is exercised via + //! [`crate::engines::set_w10_disabled_override`] — a per-thread + //! override so tests don't race other parallel tests that also + //! consult `w10_enabled()`. use larql_inference::cpu_engine_backend; use larql_inference::test_utils::{make_test_q4k_vindex, make_test_q4k_weights}; - use serial_test::serial; use super::*; use crate::engines::markov_residual::engine::MarkovResidualEngine; + use crate::engines::set_w10_disabled_override; fn fixture(window_size: Option) -> (MarkovResidualEngine, ModelWeights, VectorIndex) { let weights = make_test_q4k_weights(); @@ -292,18 +287,14 @@ mod tests { (engine, weights, index) } - /// Force `LARQL_W10_DISABLE` to a known state. Pass `true` for the - /// Full-mask path; `false` for the cascade-on default. + /// Pin W10 cascade state for this thread. `true` simulates + /// `LARQL_W10_DISABLE=1` (Full mask); `false` simulates the + /// default-on cascade (HOnly / None masks). fn set_w10_disable(disabled: bool) { - if disabled { - std::env::set_var("LARQL_W10_DISABLE", "1"); - } else { - std::env::remove_var("LARQL_W10_DISABLE"); - } + set_w10_disabled_override(Some(disabled)); } #[test] - #[serial] fn try_prefill_via_dispatch_returns_none_when_index_lacks_direct_matvec() { set_w10_disable(false); let weights = make_test_q4k_weights(); @@ -323,7 +314,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowed_keeps_stored_under_w10_default() { set_w10_disable(false); // window=Some + default env → drop_hot_kv_shadow=true, @@ -350,7 +340,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowless_drops_stored_shadow_under_w10() { set_w10_disable(false); // window=None + default env → both drop_hot_kv_shadow and @@ -370,7 +359,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_full_mask_path_with_w10_disabled() { // LARQL_W10_DISABLE=1 → drop_hot_kv_shadow=false; both shadows // populated (Full mask in decode). @@ -385,7 +373,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_without_prefill_returns_none() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(Some(4)); @@ -396,7 +383,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_windowed_appends_h_in_under_honly() { set_w10_disable(false); // window=Some + default env → mask=HOnly. hot_len grows by 1; @@ -417,7 +403,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_windowless_uses_none_mask() { set_w10_disable(false); // window=None + default env → mask=None; stored stays empty, @@ -440,7 +425,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_full_mask_appends_hot_kv_with_w10_disabled() { // Full mask path: appends to BOTH stored and hot_kv on every layer. set_w10_disable(true); @@ -458,7 +442,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_with_profiling_records_stages() { set_w10_disable(false); let (engine, mut weights, index) = fixture(Some(8)); diff --git a/crates/larql-kv/src/engines/markov_residual_codec/dispatch.rs b/crates/larql-kv/src/engines/markov_residual_codec/dispatch.rs index 6aade32ed..ebd0a6a7f 100644 --- a/crates/larql-kv/src/engines/markov_residual_codec/dispatch.rs +++ b/crates/larql-kv/src/engines/markov_residual_codec/dispatch.rs @@ -279,7 +279,6 @@ mod tests { use larql_inference::cpu_engine_backend; use larql_inference::test_utils::{make_test_q4k_vindex, make_test_q4k_weights}; - use serial_test::serial; use super::*; use crate::engines::markov_residual_codec::codec::ColdResidualCodec; @@ -303,15 +302,10 @@ mod tests { } fn set_w10_disable(disabled: bool) { - if disabled { - std::env::set_var("LARQL_W10_DISABLE", "1"); - } else { - std::env::remove_var("LARQL_W10_DISABLE"); - } + crate::engines::set_w10_disabled_override(Some(disabled)); } #[test] - #[serial] fn try_prefill_via_dispatch_returns_none_when_index_lacks_direct_matvec() { set_w10_disable(false); let weights = make_test_q4k_weights(); @@ -335,7 +329,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowed_keeps_stored_under_w10_default() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(Some(8)); @@ -353,7 +346,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowless_drops_stored_under_w10() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(None); @@ -370,7 +362,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_windowed_overflow_codecs_cold_tier() { // window=2 against 4 tokens → prefill clip evicts 2 positions // into `cold_encoded` via the bf16 codec. @@ -388,7 +379,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_full_mask_with_w10_disabled() { set_w10_disable(true); let (mut engine, mut weights, index) = fixture(Some(8)); @@ -401,7 +391,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_without_prefill_returns_none() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(Some(4)); @@ -411,7 +400,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_windowed_appends_h_in_under_honly() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(Some(8)); @@ -430,7 +418,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_windowless_uses_none_mask() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(None); @@ -450,7 +437,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_full_mask_with_w10_disabled() { set_w10_disable(true); let (mut engine, mut weights, index) = fixture(Some(8)); @@ -467,7 +453,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_overflow_extends_cold_encoded() { // window=2: after prefilling 2 → one decode evicts the oldest // position into cold_encoded (the None match arm constructs it). @@ -508,7 +493,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_with_profiling_records_stages() { set_w10_disable(false); let (engine, mut weights, index) = fixture(Some(8)); diff --git a/crates/larql-kv/src/engines/mod.rs b/crates/larql-kv/src/engines/mod.rs index a9632fe29..a1e2d79d3 100644 --- a/crates/larql-kv/src/engines/mod.rs +++ b/crates/larql-kv/src/engines/mod.rs @@ -73,6 +73,29 @@ pub mod unlimited_context; /// boundary_per_layer). Engines that treat K/V as canonical state /// (turbo_quant) don't call this — their dispatch path stays on /// Full mask regardless. +/// +/// Tests inject a value through `set_w10_disabled_override` (per-thread) +/// rather than mutating the process env, so they don't race other +/// parallel tests that also call this helper. pub(crate) fn w10_enabled() -> bool { - std::env::var("LARQL_W10_DISABLE").as_deref() != Ok("1") + let overridden = W10_DISABLED_OVERRIDE.with(|o| *o.borrow()); + match overridden { + Some(disabled) => !disabled, + None => std::env::var("LARQL_W10_DISABLE").as_deref() != Ok("1"), + } +} + +std::thread_local! { + /// Per-thread override for [`w10_enabled`]. `Some(true)` simulates + /// `LARQL_W10_DISABLE=1` (cascade off); `Some(false)` simulates the + /// var unset (cascade on); `None` falls through to the real env. + /// Test-only escape hatch — production callers leave it `None`. + static W10_DISABLED_OVERRIDE: std::cell::RefCell> = const { + std::cell::RefCell::new(None) + }; +} + +#[cfg(test)] +pub(crate) fn set_w10_disabled_override(disabled: Option) { + W10_DISABLED_OVERRIDE.with(|o| *o.borrow_mut() = disabled); } diff --git a/crates/larql-kv/src/engines/unlimited_context/dispatch.rs b/crates/larql-kv/src/engines/unlimited_context/dispatch.rs index 60c866f3c..09f93b431 100644 --- a/crates/larql-kv/src/engines/unlimited_context/dispatch.rs +++ b/crates/larql-kv/src/engines/unlimited_context/dispatch.rs @@ -164,13 +164,13 @@ impl UnlimitedContextEngine { mod tests { //! Coverage for the W1-GPU dispatch path. `UnlimitedContextEngine` //! takes a non-optional `window_size: usize`; W10 mask cascade is - //! gated on whether `current_window_kv` is dropped (the - //! `LARQL_W10_DISABLE` env var). Tests are `#[serial]` because - //! `w10_enabled()` reads a process-global env var. + //! gated on whether `current_window_kv` is dropped. Tests pin the + //! cascade state via [`crate::engines::set_w10_disabled_override`] + //! — a per-thread override so they don't race other parallel tests + //! that also call `w10_enabled()`. use larql_inference::cpu_engine_backend; use larql_inference::test_utils::{make_test_q4k_vindex, make_test_q4k_weights}; - use serial_test::serial; use super::*; use crate::engines::unlimited_context::engine::UnlimitedContextEngine; @@ -183,15 +183,10 @@ mod tests { } fn set_w10_disable(disabled: bool) { - if disabled { - std::env::set_var("LARQL_W10_DISABLE", "1"); - } else { - std::env::remove_var("LARQL_W10_DISABLE"); - } + crate::engines::set_w10_disabled_override(Some(disabled)); } #[test] - #[serial] fn try_prefill_via_dispatch_returns_none_when_index_lacks_direct_matvec() { set_w10_disable(false); let weights = make_test_q4k_weights(); @@ -211,7 +206,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_drops_window_kv_under_w10_default() { // W10 on by default → drop_window_kv_shadow = true. The engine // doesn't populate `current_window_kv`; Metal's kv cache is the @@ -230,7 +224,6 @@ mod tests { } #[test] - #[serial] fn try_prefill_via_dispatch_populates_window_kv_with_w10_disabled() { // W10 off → engine pre-allocates `[window_cap, kv_dim]` per // layer and copies the prefill K/V rows in. @@ -250,7 +243,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_without_prefill_returns_none() { set_w10_disable(false); let (mut engine, mut weights, index) = fixture(4); @@ -261,7 +253,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_h_only_skips_kv_append() { // W10 default + window_kv dropped at prefill → HOnly mask. // The dispatch decode runs through the `if !matches!(mask, HOnly)` @@ -285,7 +276,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_full_mask_appends_kv_with_w10_disabled() { // W10 off → Full mask. Each layer's K/V row blits into the // pre-allocated window_kv buffer at `current_window_kv_len`. @@ -313,7 +303,6 @@ mod tests { } #[test] - #[serial] fn decode_step_via_dispatch_fires_window_auto_close() { // window=2: prefill 2 → token count == window → close_window // fires inside the dispatch decode and resets the window.