# fluxa-core robustness & performance plan Goal: move the crate toward "if it compiles, it doesn't break at runtime" and make the hot paths (dispatch, effect completion, stream ranking) cheap enough to never show up in a profile. This document is the result of a full review of the crate as of 2026-07-07 (master @ f00a09a) and lays out what's already good, where runtime breakage can still hide, and a prioritized plan to close each gap. ## Current state, honestly assessed What's already strong and should be preserved: - `headless_engine` state is fully typed — `EngineState` contains zero `serde_json::Value` fields at the top level (`state.rs`), cross-module mutation goes through `pub(super)` setters, and generations use the `GenerationKey` enum. - Every JNI entry point wraps its body in `catch_unwind`, and `core_invoke` does too, so a panic can't cross the FFI boundary and abort the Android process. - Production code is nearly panic-free: outside test modules the only `expect` is a static regex in `addon_store.rs:748`, which can't fail for a fixed pattern. - Poisoned-mutex recovery exists (`lock_engines` uses `into_inner()`), so one panicking dispatch can't brick every later call. - `StatePatch::diff` sends only changed domains over the wire instead of the full state. - Fuzz targets exist (`episode_matching`, `manifest_parse`, `percent_decode`). - Effect delivery has dedup (`delivered_effect_ids`) and expiry (`EFFECT_EXPIRY`), so a lost `completeEffect` doesn't leak or double-execute. What undermines the "compiles ⇒ works" goal, in order of real-world risk: ## P0 — fix immediately ### 1. Master has a failing test `player_policy::tests::convert_dv81_hls_still_deferred_to_manifest_rewrite` fails. The branch at `player_policy.rs:445` returns `hls_rpu_convert` for HLS + P7 + `convert_dv81` + DV decoder, while the test still expects `none`/`manifest_handled`. Either the new branch is a policy change and the test is stale, or the branch regressed the "HLS is always the OkHttp interceptor's job" rule that `sample_hls_stream_always_deferred_to_manifest_rewrite` documents. Decide which, fix, and add a CI gate (see §8) so a red master can't happen silently again. ## P1 — silent failures at the boundary ### 2. `Option` swallows every error The dominant FFI pattern is `*_json(&str) -> Option` (~295 functions), almost always built on `serde_json::from_str(...).ok()?` (~135 sites). Any malformed or schema-drifted input produces `None` → the host receives `null` with zero diagnostics. This is the single biggest source of "compiles but breaks at runtime": a Kotlin-side field rename doesn't crash anything, it just makes a feature quietly stop working. Plan, incremental and wire-compatible: - Introduce one internal error type: ```rust pub(crate) enum CoreError { BadInput { context: &'static str, detail: String }, NotFound { context: &'static str }, } ``` Migrate module internals from `Option` to `Result`, mechanical module by module (`?` still works). Keep the outer `Option` shims so JNI signatures and the Kotlin side don't change. - At each boundary shim, on `Err` route the detail through a host-pluggable logger before returning `None`/`null`. Add a `core.setLogSink` (JNI: one extern that stores a JVM callback; desktop: a `fn(String)` setter) so failures land in logcat / desktop logs instead of vanishing. This turns every future wire drift from "feature silently dead" into a one-line log with the exact field that failed to parse. - `core_invoke` already has an error envelope — have its shims return the real `CoreError` kind/message instead of collapsing to `NotFound` via `Option`. ### 3. `EffectResultInput` accepts garbage as success Every field is `#[serde(default)]` (`contracts.rs:318`), so a completely wrong-shaped `completeEffect` payload deserializes into `effect_id: ""`, `status: ""` — the engine then looks up effect `""`, finds nothing, and the completion is dropped without a trace. Make `effect_id` required, make `status` an enum: ```rust #[serde(rename_all = "camelCase")] enum EffectStatus { Ok, Error, Cancelled } ``` Reject unknown statuses at parse time with a logged error (via §2's sink) instead of treating them as an empty string that matches no arm. ## P2 — make invalid states unrepresentable ### 4. `Value` leaves inside the typed engine `EngineState` is typed, but the payloads flowing through it are not: ~370 `Value` / `Vec` / `Option` occurrences across `headless_engine/*.rs`. The repeat offenders in `AppAction` are `profile: Option`, `meta: Value`, `streams: Vec`, `item: Value`. Every `.get("...").and_then(as_str)` chain on these is a stringly-typed runtime contract the compiler can't check. Plan: define the three or four shared wire types once, in a new `types/wire.rs` (the `types/` module dir already exists), with `#[serde(rename_all = "camelCase")]` and `#[serde(default)]` on genuinely optional fields: - `Profile` — `profile_prefs.rs` already does a typed read of the same JSON; promote that struct instead of writing a new one. - `MetaItem` / `Video` — the detail/library/watchlist modules all re-derive the same fields (`id`, `type`, `name`, `poster`, `videos`, episode fields) from `Value`. - `Stream` — `stream_policy.rs` already has typed stream structs; reuse them in `AppAction::PlayerStreamsLoaded` and friends. Crucial constraint: any field the platform sends that Rust doesn't model must survive round-trips, because effect payloads echo these objects back. Use `#[serde(flatten)] extra: serde_json::Map` on each wire type so unknown fields pass through unchanged. This lets migration happen field-by-field without ever breaking a consumer that sends more than we model. Migrate one action at a time; the JSON on the wire never changes, so this is invisible to the platforms. Priority order by breakage frequency: `profile` (touched by ~20 actions), `meta`, `streams`, `item`. ### 5. `json!` payload literals defeat compile-time checking The stated convention is "effect payloads are small `#[derive(Serialize)]` structs", but `headless_engine/` still has ~80 `json!({...})` payload literals. A typo'd key in one of those compiles fine and the platform-side effect executor gets a field it doesn't recognize. Finish the migration: one payload struct per effect kind, named after it (`FetchDetailStreamsPayload`), living next to the module that emits it. Once done, add `json!` to the review checklist / grep gate for `headless_engine/` (it stays fine in tests and in `ffi.rs` envelope construction). ### 6. Stringly-typed enums that already have a closed set - `EffectEnvelope.kind` is `String` even though `EffectKind` exists and has an exhaustive test. Change the field to `EffectKind` with `#[derive(Serialize, Deserialize)] #[serde(rename_all = "camelCase")]` on the enum — serde then generates exactly the strings `as_str()` produces today, the 100-line hand-written `as_str`/`from_str` pair and its roundtrip test get deleted, and adding a variant can no longer be forgotten in one of three places. `EffectEnvelope::raw` has to go or become test-only; audit its callers first. - `fallback_mode` in `player_policy.rs` is compared as `"convert_dv81"` / `"auto"` / `"off"` string literals throughout `dv_proxy_plan_json`. Parse once into an enum at the request boundary; a typo in a mode string then fails at the one parse site, not silently in whichever comparison spelled it wrong. - Same treatment for `source_selection_mode`, scrobble `action_name`, and auth `provider`/`mode` — each is a closed set spread across string comparisons today. ## P3 — the three uncoordinated FFI surfaces ### 7. One method inventory instead of three hand-wired ones `ffi.rs` routes ~115 method names by string; `jni.rs` hand-wires ~105 externs; nothing guarantees the same domain function is exposed consistently or spelled the same. The CLAUDE.md assessment that a full consolidation isn't worth the rewrite risk stands — but a cheap middle ground removes most drift without touching `jni.rs`'s hot path: - Declare the string-routed surface in one table macro: ```rust core_methods! { "streamPolicy.rankStreams" => stream_policy::rank_streams_json(json), "engine.dispatch" => (handle: u64, action: str) headless_engine::headless_engine_dispatch_json, ... } ``` generating the `route_*` functions in `ffi.rs`. Adding a method becomes one line; the per-domain router boilerplate and its UnknownMethod-chaining protocol disappear. - Generate `core_capabilities` from the same table so the platform can feature-detect instead of calling methods that may not exist in an older core build. - Leave `jni.rs` as-is (it's Android's proven path), but add a unit test that walks the table and asserts every `core_invoke` method name a consumer repo references (kept in a checked-in fixture list) still routes. That converts "renamed a method string" from an Android-runtime discovery into a test failure here. ## P4 — cross-repo wire contract ### 8. The wire format has no executable contract Field names in `dispatch`/`completeEffect`/effect payloads are read by Kotlin/TS in other repos; today the only protection is care. Two mechanisms, both cheap: - **Golden fixtures**: a `tests/wire/` directory of checked-in JSON files — one real `AppAction` input and one expected `DispatchResult` output per action family, plus one payload sample per `EffectKind`. A snapshot test deserializes, dispatches, and compares serialized output structurally. Any refactor that changes camelCase output now fails a test in *this* repo instead of breaking Android in production. When a change is intentional, the diff in the fixture file is the review artifact the platform PRs can be checked against. - **Fuzz the real boundary**: the existing fuzz targets cover parsers, but the highest- value target is missing — `headless_engine_dispatch_json` + `complete_effect_json` with arbitrary bytes and with structure-aware mutation of valid actions. The engine holding global state behind `catch_unwind`-free internals (dispatch is only guarded at the JNI layer, not in the desktop path via direct calls) makes this the place a panic or logic corruption would actually hurt. Add `engine_dispatch.rs` to `fuzz/fuzz_targets/`. ## P5 — performance The release profile is already right (`lto = true`, `codegen-units = 1`, opt-level 3). Keep `panic = "unwind"` — the FFI `catch_unwind` guards depend on it; `panic = "abort"` would turn every guarded panic into a process kill. The actual costs found: ### 9. Two full state clones per dispatch `headless_engine_dispatch_json` (`mod.rs:88-100`) clones the entire `EngineState` before the reducer runs, clones it again after, then `StatePatch::diff` does a deep `PartialEq` walk of all 16 domains and clones each changed one a third time. Cost scales with everything the user has ever loaded (full catalogs, episode lists), not with what the action touched — on a TV device after a long session this is the dispatch latency. Fix without changing the wire format: replace clone-and-diff with dirty tracking. Each domain module already owns its state behind `pub(super)` setters — the setters are the natural single place to flip a `dirty: DomainMask` bit on the engine. `StatePatch` then clones only domains whose bit is set, and the before-clone disappears entirely. `PartialEq` on the domain structs stays for tests. This is the single highest-leverage performance change in the crate; do it before micro-work. Second-order: `snapshot_json` and dispatch serialize while holding the global engines mutex. Move `serde_json::to_string` outside the lock (clone the patch domains inside, serialize after release) so a slow serialization of a big home state can't block a concurrent `completeEffect`. ### 10. Per-call regex compilation in ranking loops `stream_policy.rs:548,594,608` build a `Regex` from the user's language preference on every call — and these run inside per-stream ranking. `external_sync.rs:65` compiles `tt\d+` per history item; `watchlist_plan.rs:216` compiles a static GitHub-URL regex per call. `content_identity.rs:980` already shows the house pattern (`OnceLock`). Statics move to `OnceLock`; the preference-derived ones get a one-entry cache keyed by the preference string (a `Mutex>` is enough — the preference changes once per settings edit, not per stream). ### 11. Desktop pays JSON tax it doesn't owe — RESOLVED, already fixed on the desktop side Verified against `fluxa-desktop/src-tauri` call sites (2026-07) per the CLAUDE.md rule, and this section's premise no longer holds. The hottest paths — `dispatchAction`, `completeEffect`, `getSnapshot` in `src/core/engine.ts` — already go through dedicated Tauri commands (`engine_dispatch`, `engine_complete_effect`, `engine_snapshot` in `src-tauri/src/lib.rs`) that call `FluxaCore::headless_engine_dispatch_json` etc. directly in Rust, not through `core_invoke`. `FluxaCore` has also grown from the "8 methods" this doc assumed to 21 (cast/AirPlay/Chromecast/Roku helpers included), all with real desktop call sites. The one remaining `core_invoke` Tauri command is a generic fallback for the long tail of infrequent methods (`playbackPreparePlan`, `preferencesSchema`, etc.) — writing a dedicated Tauri command per method there would be pure boilerplate, since Tauri IPC already serializes args as JSON crossing the JS↔Rust boundary regardless of whether the command is generic or dedicated; there's no double-parse to eliminate. Nothing left to do here. ### 12. Measure before and after Add a `benches/` with criterion (dev-dependency only): dispatch on a realistic large state (fixture from §8's golden files), `StatePatch::diff` vs dirty-tracking, stream ranking with 500 streams. Without these, §9-§11 are guesses; with them, regressions show up in review. ## P6 — enforcement so it stays fixed ### 13. Lint and CI gates There is no CI in this repo and no lint policy in `Cargo.toml`. Add: ```toml [lints.rust] unsafe_op_in_unsafe_fn = "warn" [lints.clippy] unwrap_used = "warn" expect_used = "warn" indexing_slicing = "warn" panic = "warn" ``` `warn` not `deny` initially — test modules use these legitimately; add `#![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used))]` and ratchet to `deny` once the count is zero. CI (GitHub Actions, single workflow): `cargo test --lib`, `cargo clippy -- -D warnings` on default features, `cargo check --no-default-features --features wasm` (allowing its known dead-code noise per CLAUDE.md), and `cargo fmt --check`. The wasm job is what catches "compiles on Android, broken on webOS" cfg mistakes. ## Sequencing | Order | Work | Sections | Size | |---|---|---|---| | 1 | Fix failing DV test, stand up CI | §1, §13 | small | | 2 | Golden wire fixtures + dispatch fuzz target | §8 | small-medium | | 3 | `CoreError` + log sink at boundaries | §2, §3 | medium, mechanical | | 4 | Dirty-flag `StatePatch`, serialize outside lock, benches | §9, §12 | medium | | 5 | Typed `Profile`/`MetaItem`/`Stream` wire structs | §4 | large, incremental | | 6 | `EffectKind` on envelope, mode enums, payload structs | §5, §6 | medium | | 7 | `core_methods!` table + capability generation | §7 | medium | | 8 | Desktop typed fast path | §11 | small | Steps 2 and 3 are what make step 5 safe: once golden fixtures exist and parse failures are logged instead of swallowed, converting `Value` fields to typed structs can be done aggressively — any drift shows up as a fixture diff or a logged parse error instead of a silent feature death on a platform we can't see from this repo.