# Final Fixes Before H100 — Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Fix all 6 remaining issues blocking H100 deployment: train/eval mismatch, NaN objective, broken trial budget, hardcoded CVaR threshold, duplicate trade threshold, stale test vectors.

**Architecture:** The root cause chain is: broken trial budget → too many evals → backtest without hold enforcement → NaN Sharpe → 1e6 penalty. Fix from bottom up: (1) add hold_time tracking to backtest kernel, (2) wire hold enforcement into evaluator's action_select, (3) fix trial budget observer, (4) make remaining hardcoded values dynamic.

**Tech Stack:** CUDA kernel (C), Rust (cudarc 0.19.3), ml-hyperopt crate.

---

## Task 1: Add hold_time tracking to backtest_env_kernel.cu

**Files:**
- Modify: `crates/ml/src/cuda_pipeline/backtest_env_kernel.cu`
- Modify: `crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs`

The backtest kernel's `portfolio_buf` is stride-8 (8 floats/window). The `experience_action_select` kernel reads `ps[10]` (hold_time) at stride-20. These are incompatible. The simplest fix: add `hold_time` as field [5] in the backtest portfolio (currently `[5]`, `[6]`, `[7]` are unused/zero).

- [ ] **Step 1: Add hold_time tracking to backtest_env_step kernel**

In `backtest_env_kernel.cu`, find the portfolio state layout. Currently:
```
[0] value, [1] position, [2] cash, [3] unrealised_pnl, [4] max_equity, [5-7] unused
```

Add hold_time tracking at index [5]:
```cuda
// Read hold_time from portfolio state
float hold_time = portfolio[w * 8 + 5];

// After position update, track hold_time
if (fabsf(new_position) > 0.001f && fabsf(prev_position) > 0.001f) {
    // Same direction — increment hold
    int prev_s = (prev_position > 0.001f) ? 1 : -1;
    int curr_s = (new_position > 0.001f) ? 1 : -1;
    if (prev_s == curr_s) {
        hold_time += 1.0f;
    } else {
        hold_time = 1.0f;  // Reversal — new trade starts
    }
} else if (fabsf(new_position) > 0.001f && fabsf(prev_position) < 0.001f) {
    hold_time = 1.0f;  // Entry from flat
} else {
    hold_time = 0.0f;  // Flat
}

// Write back
portfolio[w * 8 + 5] = hold_time;
```

- [ ] **Step 2: Add min_hold_bars hold enforcement to backtest_env_step**

Add `int min_hold_bars` as a kernel parameter. After computing new_position but before applying it:

```cuda
// Hold enforcement (Layer 2 — same as training kernel)
int prev_sign = (prev_position > 0.001f) ? 1 : ((prev_position < -0.001f) ? -1 : 0);
int curr_sign = (new_position > 0.001f) ? 1 : ((new_position < -0.001f) ? -1 : 0);
int wants_exit = (prev_sign != 0 && curr_sign == 0);
int wants_reversal = (prev_sign != 0 && curr_sign != 0 && prev_sign != curr_sign);

if (hold_time > 0.0f && hold_time < (float)min_hold_bars
    && (wants_exit || wants_reversal)
    && step < window_len - 1) {
    new_position = prev_position;  // Override: keep position
}
```

- [ ] **Step 3: Pass min_hold_bars from Rust**

In `gpu_backtest_evaluator.rs`:
- Add `min_hold_bars: i32` to `GpuBacktestConfig` (default: 5)
- Pass to `backtest_env_step` kernel launch as the last arg
- Wire from hyperopt adapter: `config.min_hold_bars = hyperparams.min_hold_bars as i32`

- [ ] **Step 4: Build + test**

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`
Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- backtest`

- [ ] **Step 5: Commit**

```bash
git commit -m "feat: hold_time tracking + hold enforcement in backtest_env_step

Backtest portfolio now tracks hold_time at index [5]. min_hold_bars
param enforces hold constraint during evaluation, matching training.
Fixes train/eval mismatch that caused NaN Sharpe → 1e6 penalty."
```

---

## Task 2: Wire hold enforcement into evaluator's action_select

**Files:**
- Modify: `crates/ml/src/cuda_pipeline/gpu_backtest_evaluator.rs`

Now that the backtest kernel tracks hold_time at `portfolio[w*8+5]`, we need a stride-20 adapter buffer for `experience_action_select`. But actually, since the evaluator uses greedy argmax (epsilon=0), the action masking is simpler — we can just let the backtest_env_step hold enforcement (Layer 2 from Task 1) handle it. The action_select kernel doesn't NEED hold masking during eval because:
1. Epsilon = 0 (greedy, no random exploration to mask)
2. The env_step kernel overrides any hold-violating action anyway

So we keep `null_portfolio/0/0` in the action_select launch but rely on the backtest_env_step hold enforcement from Task 1 to enforce holds. This avoids the stride-8/stride-20 incompatibility.

- [ ] **Step 1: Update the comment to document the design decision**

Replace the comment at the evaluator's action_select launch:
```rust
let null_portfolio: u64 = 0;  // Hold enforcement handled by backtest_env_step (Layer 2)
let eval_min_hold: i32 = 0;   // Action masking disabled — greedy argmax + env override sufficient
let eval_max_pos: f32 = 0.0;
```

- [ ] **Step 2: Build + test**

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`

- [ ] **Step 3: Commit**

```bash
git commit -m "docs: clarify evaluator action masking strategy — Layer 2 in env_step handles holds"
```

---

## Task 3: Fix trial budget observer

**Files:**
- Modify: `crates/ml-hyperopt/src/observer.rs`
- Modify: `crates/ml-hyperopt/src/optimizer.rs`

The `TrialBudgetObserver.increment_trial()` is never called. The budget counter stays at 0 forever. With `num_trials=2`, PSO runs 20 particle evaluations instead of 1.

- [ ] **Step 1: Call `increment_trial()` from `evaluate_point()`**

In `optimizer.rs`, find `evaluate_point()` (or the function that calls `model.train_with_params()`). After each evaluation completes, increment the observer's trial counter.

The challenge: `TrialBudgetObserver` is passed to `argmin::Executor` and the optimizer doesn't have direct access to it during evaluation. The fix: use an `Arc<AtomicUsize>` shared between the observer and the cost function.

```rust
// In ArgminOptimizer::optimize():
let trial_counter = Arc::new(AtomicUsize::new(0));

// Pass to ObjectiveFunction:
let cost_fn = ObjectiveFunction {
    model: adapter,
    trial_counter: trial_counter.clone(),
    max_trials: self.max_trials,
};

// Pass to TrialBudgetObserver:
let observer = TrialBudgetObserver::new(self.max_trials, trial_counter.clone());

// In ObjectiveFunction::cost():
fn cost(&self, param: &Vec<f64>) -> Result<f64, Error> {
    let trial_num = self.trial_counter.fetch_add(1, Ordering::SeqCst);
    if trial_num >= self.max_trials {
        return Err(Error::msg("Trial budget exhausted"));
    }
    // ... train_with_params ...
}
```

- [ ] **Step 2: Update TrialBudgetObserver to use shared counter**

```rust
pub struct TrialBudgetObserver {
    max_trials: usize,
    trial_counter: Arc<AtomicUsize>,
}

impl TrialBudgetObserver {
    pub fn new(max_trials: usize, trial_counter: Arc<AtomicUsize>) -> Self {
        Self { max_trials, trial_counter }
    }
}

impl<I> Observe<I> for TrialBudgetObserver {
    fn observe_iter(&mut self, _state: &I, _kv: &KV) -> Result<(), Error> {
        let used = self.trial_counter.load(Ordering::SeqCst);
        if used >= self.max_trials {
            Err(Error::msg(format!("Trial budget exhausted: {used}/{}", self.max_trials)))
        } else {
            Ok(())
        }
    }
}
```

- [ ] **Step 3: Build + test**

Run: `SQLX_OFFLINE=true cargo test -p ml-hyperopt --lib`
Expected: optimizer tests pass, trial count is respected

- [ ] **Step 4: Commit**

```bash
git commit -m "fix: trial budget observer — shared AtomicUsize counter between cost fn and observer

increment_trial() was never called — budget enforcement was broken.
Now the ObjectiveFunction::cost() increments the shared counter on each
evaluation, and TrialBudgetObserver checks it per PSO iteration.
With num_trials=2: exactly 2 evaluations (1 LHS + 1 PSO), not 21."
```

---

## Task 4: Make CVaR threshold dynamic

**Files:**
- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs`

- [ ] **Step 1: Replace hardcoded `0.003` with dynamic computation**

Find line ~3772:
```rust
let cvar_threshold = 0.003;
```

Replace with:
```rust
// CVaR threshold scales with bar frequency: 0.05 / sqrt(bars_per_day)
// 1-min (390): 0.003, 5-min (78): 0.006, daily (1): 0.05
let bars_per_day = metrics.bars_per_day.unwrap_or(390.0);
let cvar_threshold = 0.05 / bars_per_day.sqrt();
```

This requires adding `bars_per_day` to `DQNMetrics`. If that's too invasive, use the constant from `common::thresholds::time::BARS_PER_DAY`:

```rust
let cvar_threshold = 0.05 / common::thresholds::time::BARS_PER_DAY.sqrt();
```

- [ ] **Step 2: Build + test**

Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- objective`

- [ ] **Step 3: Commit**

```bash
git commit -m "fix: CVaR threshold scales with bars_per_day — was hardcoded at 0.003"
```

---

## Task 5: Extract trade threshold constant + fix stale test vectors

**Files:**
- Modify: `crates/ml/src/hyperopt/adapters/dqn.rs`

- [ ] **Step 1: Extract named constant for trade threshold**

Find the two independent `10` values at lines ~2496 and ~3738. Replace both with:

```rust
/// Minimum trades for non-degenerate trial (below this → full penalty).
const MIN_TRADES_DEGENERATE: usize = 10;
```

Use `MIN_TRADES_DEGENERATE` in both locations.

- [ ] **Step 2: Fix stale test vectors**

Search for `vec![0.0_f64; 39]`, `vec![0.0_f64; 41]`, `vec![0.0; 45]` in test code. Update to `vec![0.0_f64; 46]` (current 46D search space) or use `DQNParams::continuous_bounds().len()` dynamically:

```rust
let dim = DQNParams::continuous_bounds().len();
let mut vec = vec![0.0_f64; dim];
```

- [ ] **Step 3: Build + test**

Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- hyperopt`

- [ ] **Step 4: Commit**

```bash
git commit -m "refactor: extract MIN_TRADES_DEGENERATE constant, fix stale 39D/41D test vectors"
```

---

## Task 6: Integration test + local hyperopt validation

- [ ] **Step 1:** `SQLX_OFFLINE=true cargo test -p ml --lib` — 887+ pass
- [ ] **Step 2:** `SQLX_OFFLINE=true cargo test -p ml-dqn --lib` — 359 pass
- [ ] **Step 3:** `SQLX_OFFLINE=true cargo test -p ml-hyperopt --lib` — all pass
- [ ] **Step 4:** `SQLX_OFFLINE=true cargo check --workspace` — clean
- [ ] **Step 5:** Local hyperopt smoke test:

```bash
rm -rf /tmp/.cubin_cache/
FOXHUNT_TEST_DATA=/home/jgrusewski/Work/foxhunt/test_data/futures-baseline \
SQLX_OFFLINE=true cargo test -p ml --lib --release -- test_local_hyperopt --ignored --nocapture
```

Expected: objective < 1000000 (not degenerate), trade count reasonable

- [ ] **Step 6:** Push to main

---

## Execution Order

```
Task 1 (backtest hold_time + enforcement) → Task 2 (document strategy)
    → Task 3 (trial budget fix) → Task 4 (CVaR dynamic)
    → Task 5 (constants + tests) → Task 6 (integration)
```

Tasks 1-2 fix the train/eval mismatch (root cause of 1e6 penalty).
Task 3 fixes the trial budget (root cause of 21 evals instead of 2).
Tasks 4-5 are cleanup.
Task 6 validates everything.