# Trade Lifecycle Fix — 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 4 trade lifecycle bugs that cause 77% turnover (620K trades / 800K bars) by adding 3 layers of defense: action masking, kernel hold override, and reward scaling. Target: <120K trades/epoch with min_hold_bars=5.

**Architecture:** Layer 1 (action masking in `experience_action_select`) prevents the agent from choosing exit/reversal during hold period. Layer 2 (kernel override in `experience_env_step`) catches any action that slips through masking. Layer 3 (reward scaling) gives smooth gradient toward longer holds. Config wires `min_hold_bars` through DQNHyperparameters → TOML → hyperopt.

**Tech Stack:** CUDA kernels (C), Rust (cudarc 0.19.3), TOML config.

**Critical path:** Only `experience_action_select` and `experience_env_step` in `experience_kernels.cu` are in the GPU-fused training loop. The `branching_action_select` in `epsilon_greedy_kernel.cu` is only used by the backtest evaluator and tests — not the training hot path.

---

## Task 1: Add `min_hold_bars` to config and wire through TOML

**Files:**
- Modify: `crates/ml/src/trainers/dqn/config.rs`
- Modify: `crates/ml/src/training_profile.rs`
- Modify: `config/training/dqn-smoketest.toml`
- Modify: `config/training/dqn-production.toml`

- [ ] **Step 1: Add `min_hold_bars` field to `DQNHyperparameters`**

In `config.rs`, after the `bars_per_day` field (~line 905), add:

```rust
    /// Minimum bars to hold a position before allowing exit or reversal.
    /// Prevents per-bar churning. Trailing stop can override after this period.
    /// Default: 5 (about 5 minutes for 1-min bars). Range: [2, 20].
    pub min_hold_bars: usize,
```

- [ ] **Step 2: Set default in `conservative()` and `Default` impl**

In `conservative()` (~line 1376 area, after `bars_per_day`):
```rust
            min_hold_bars: 5,
```

- [ ] **Step 3: Add to TOML experience section in `training_profile.rs`**

Find the `ExperienceSection` struct (or the section that handles `[experience]` TOML). Add:
```rust
    pub min_hold_bars: Option<usize>,
```

In the `apply_to()` method:
```rust
    if let Some(v) = self.min_hold_bars {
        hp.min_hold_bars = v;
    }
```

- [ ] **Step 4: Update TOML files**

In `config/training/dqn-smoketest.toml`, add under `[experience]`:
```toml
min_hold_bars = 3
```

In `config/training/dqn-production.toml`, add under `[experience]`:
```toml
min_hold_bars = 5
```

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

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`
Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- training_profile`
Expected: compiles, profile tests pass

- [ ] **Step 6: Commit**

```bash
git add crates/ml/src/trainers/dqn/config.rs crates/ml/src/training_profile.rs config/training/dqn-smoketest.toml config/training/dqn-production.toml
git commit -m "feat: add min_hold_bars config (default 5) — prevents per-bar churning"
```

---

## Task 2: Layer 2 — Fix hold enforcement in `experience_env_step` kernel

**Files:**
- Modify: `crates/ml/src/cuda_pipeline/experience_kernels.cu`
- Modify: `crates/ml/src/cuda_pipeline/gpu_experience_collector.rs`

This is the safety-net layer. Fix the dead code boolean, extend to reversals, update trailing stop threshold, and add reward scaling.

- [ ] **Step 1: Add `min_hold_bars` parameter to `experience_env_step` kernel**

In `experience_kernels.cu`, add `int min_hold_bars` to the kernel signature after the last existing parameter (`raw_returns_out`). Update the `@param` comment block.

- [ ] **Step 2: Pass `min_hold_bars` from Rust**

In `gpu_experience_collector.rs`, add `min_hold_bars: i32` to `ExperienceCollectorConfig` (note: the struct is `ExperienceCollectorConfig`, not `GpuExperienceCollectorConfig`). In the kernel launch (~line 1298-1327), add `.arg(&min_hold_bars_i32)` after the `raw_returns_out` arg. Compute `let min_hold_bars_i32 = config.min_hold_bars as i32;` before the launch. Also compute `let max_pos = config.max_position;` BEFORE the action_select launch (it's currently defined only at line 1293 for env_step — move it up so both launches can use it).

Wire it from `DQNHyperparameters`: in the constructor where config is built, set `min_hold_bars: hyperparams.min_hold_bars as i32`.

- [ ] **Step 3: Replace dead hold guard with unified hold enforcement**

In `experience_kernels.cu`, replace the block at lines 795-814 (the dead early-exit override) with:

```cuda
    /* ════════════════════════════════════════════════════════════════════
     * HOLD ENFORCEMENT: block ALL position changes during minimum hold.
     * Applies to BOTH flat exits AND reversals.
     *
     * This replaces the old guard which was dead code:
     *   `prev_sign != 0 && curr_sign == 0 && !exiting_trade` was always
     *   false because exiting_trade = (prev_sign != 0 && curr_sign == 0).
     *
     * Layer 2 safety net — Layer 1 (action masking) should prevent most
     * hold violations, but epsilon exploration can still slip through.
     * ════════════════════════════════════════════════════════════════════ */
    int wants_exit = (prev_sign != 0 && curr_sign == 0);
    int wants_reversal = (prev_sign != 0 && curr_sign != 0 && prev_sign != curr_sign);
    int hold_violation = (hold_time > 0.0f
                          && hold_time < (float)min_hold_bars
                          && (wants_exit || wants_reversal)
                          && bar_idx < total_bars - 1);  /* bypass at episode end */

    if (hold_violation) {
        /* Override: keep previous position, cancel the action */
        position = ps[0];
        cash = ps[1];
        is_flat = (fabsf(position) < 0.001f) ? 1.0f : 0.0f;

        /* Fix the stored action to match held exposure (prevent action aliasing) */
        float prev_exposure_frac = ps[0] / (max_position > 0.0f ? max_position : 1.0f);
        int held_exposure = (int)roundf((prev_exposure_frac + 1.0f) * 0.5f * (float)(b0_size - 1));
        if (held_exposure < 0) held_exposure = 0;
        if (held_exposure >= b0_size) held_exposure = b0_size - 1;
        int original_order = (action_idx / b2_size) % b1_size;
        int original_urgency = action_idx % b2_size;
        action_idx = held_exposure * b1_size * b2_size + original_order * b2_size + original_urgency;
        out_actions[out_off] = action_idx;

        /* Recompute signs from overridden position */
        curr_sign = prev_sign;
        wants_exit = 0;
        wants_reversal = 0;
    }

    /* Compute final trade lifecycle from (possibly overridden) state */
    int exiting_trade = wants_exit;
    int reversing_trade = wants_reversal;
    int segment_complete = exiting_trade || reversing_trade;
```

Also REMOVE the old lines 716-720 (`int exiting_trade`, `int reversing_trade`, `int segment_complete`) since they move into the new block. Keep `entering_trade` at line 715 as it's not affected. Add a preliminary `exiting_trade` variable that the trailing stop block at line 790 can write to:

```cuda
    int entering_trade  = (prev_sign == 0 && curr_sign != 0);
    int exiting_trade = 0;  /* preliminary — trailing stop may set to 1, hold guard recomputes */
```

The trailing stop at line 790 sets `exiting_trade = 1;` — this variable must exist before the trailing stop block runs. The hold enforcement block below then recomputes it from the (possibly overridden) position.

- [ ] **Step 4: Update trailing stop threshold**

Change line 780 from:
```cuda
    if (prev_sign != 0 && hold_time > 2.0f && peak_equity > 1.0f) {
```
to:
```cuda
    if (prev_sign != 0 && hold_time >= (float)min_hold_bars && peak_equity > 1.0f) {
```

The trailing stop only fires after the hold period completes.

- [ ] **Step 5: Add reward scaling for short trades (Layer 3)**

After the existing reward computation (around line 927 where `reward` is fully computed), add:

```cuda
    /* Layer 3: Reward scaling — shorter trades get proportionally less reward.
     * Smooth gradient toward longer holds (vs binary cliff at min_hold).
     * Floor at 10% to maintain gradient signal for very short trades
     * that slip through (episode boundary, trailing stop override). */
    if (segment_complete && segment_hold_time > 0.0f && min_hold_bars > 0) {
        float hold_scale = fminf(segment_hold_time / (float)min_hold_bars, 1.0f);
        hold_scale = fmaxf(hold_scale, 0.1f);
        reward *= hold_scale;
    }
```

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

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`
Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- backtest`
Expected: compiles, backtest tests pass (kernel PTX compiles)

- [ ] **Step 7: Commit**

```bash
git add crates/ml/src/cuda_pipeline/experience_kernels.cu crates/ml/src/cuda_pipeline/gpu_experience_collector.rs
git commit -m "fix: hold enforcement in experience_env_step — blocks exits AND reversals during min_hold

Layer 2 (safety net): unified hold guard replaces dead code boolean.
Old guard (prev_sign!=0 && curr_sign==0 && !exiting_trade) was always
false. New guard covers both flat exits and reversals.

Layer 3 (gradient): reward scaled by hold_time/min_hold_bars (floor 10%).
Trailing stop respects min_hold_bars (was hardcoded > 2.0f)."
```

---

## Task 3: Layer 1 — Action masking in `experience_action_select`

**Files:**
- Modify: `crates/ml/src/cuda_pipeline/experience_kernels.cu`
- Modify: `crates/ml/src/cuda_pipeline/gpu_experience_collector.rs`

This is the primary defense layer — prevents the agent from even choosing exit/reversal during the hold period.

- [ ] **Step 1: Add parameters to `experience_action_select` kernel**

In `experience_kernels.cu`, update the `experience_action_select` kernel signature to add portfolio state access:

```cuda
extern "C" __global__ void experience_action_select(
    const float*  __restrict__ q_values,
    int*                       out_actions,
    unsigned int*              rng_states,
    float*                     out_q_gaps,
    float epsilon,
    int N,
    int b0_size,
    int b1_size,
    int b2_size,
    float q_gap_threshold,
    const float* __restrict__ portfolio_states,  /* NEW: [N, PORTFOLIO_STRIDE] */
    int min_hold_bars,                            /* NEW */
    float max_position                            /* NEW */
) {
```

- [ ] **Step 2: Add action masking logic**

After `int a0, a1, a2;` (line 374), add the hold-time check and masking:

```cuda
    /* Hold enforcement: during minimum hold period, force current exposure.
     * Prevents both greedy and random exploration from changing exposure.
     * Portfolio state: ps[0] = position, ps[10] = hold_time. */
    int ps_base = i * 20;  /* PORTFOLIO_STRIDE = 20 */
    float hold_time = portfolio_states[ps_base + 10];
    float cur_position = portfolio_states[ps_base + 0];
    int in_hold = (hold_time > 0.0f && hold_time < (float)min_hold_bars
                   && min_hold_bars > 0);

    if (in_hold) {
        /* Compute current exposure index from position */
        float exposure_frac = cur_position / (max_position > 0.0f ? max_position : 1.0f);
        int cur_exp = (int)roundf((exposure_frac + 1.0f) * 0.5f * (float)(b0_size - 1));
        if (cur_exp < 0) cur_exp = 0;
        if (cur_exp >= b0_size) cur_exp = b0_size - 1;
        a0 = cur_exp;  /* Force current exposure — skip Q-value selection entirely */
        /* Zero Q-gap during hold — conviction sizing is meaningless when forced */
        if (out_q_gaps != NULL) {
            out_q_gaps[i] = 0.0f;
        }
    } else {
```

Then wrap the existing exposure selection (both epsilon and greedy paths) in the `else` branch, and close with `}`.

- [ ] **Step 3: Pass portfolio_states to kernel launch**

In `gpu_experience_collector.rs`, update the action_select kernel launch (~line 1243-1259) to add the three new args:

```rust
                    .arg(&self.portfolio_states)    // portfolio state for hold masking
                    .arg(&min_hold_bars_i32)         // min_hold_bars
                    .arg(&max_pos)                   // max_position (for exposure index)
```

Add these after `.arg(&q_gap)`.

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

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`
Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- backtest`
Expected: compiles, all tests pass

- [ ] **Step 5: Commit**

```bash
git add crates/ml/src/cuda_pipeline/experience_kernels.cu crates/ml/src/cuda_pipeline/gpu_experience_collector.rs
git commit -m "feat: action masking in experience_action_select — Layer 1 hold enforcement

During min_hold period, forces exposure to current position index.
Covers both greedy (Q-value) and random (epsilon) paths.
Portfolio state already GPU-resident — zero-copy parameter pass."
```

---

## Task 4: Add `min_hold_bars` to hyperopt search space

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

- [ ] **Step 1: Add `min_hold_bars` to `DQNParams`**

Find the `DQNParams` struct and add:
```rust
    pub min_hold_bars: usize,
```

In `Default`:
```rust
    min_hold_bars: 5,
```

- [ ] **Step 2: Add to search space bounds**

In `continuous_bounds()`, add a new dimension:
```rust
    bounds.push((2.0, 20.0));  // min_hold_bars [2, 20]
```

In `param_names()`:
```rust
    names.push("min_hold_bars");
```

- [ ] **Step 3: Wire in `from_continuous()` and `to_continuous()`**

In `from_continuous()`:
```rust
    params.min_hold_bars = continuous[IDX].round() as usize;
    params.min_hold_bars = params.min_hold_bars.clamp(2, 20);
```

In `to_continuous()`:
```rust
    vec.push(self.min_hold_bars as f64);
```

- [ ] **Step 4: Wire to `build_hyperparams()`**

In `build_hyperparams()` where `DQNHyperparameters` is constructed:
```rust
    hp.min_hold_bars = params.min_hold_bars;
```

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

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`
Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- hyperopt`
Expected: all hyperopt tests pass (search space dimension increased by 1)

- [ ] **Step 6: Update test assertions for new search space dimension**

Any test that asserts the exact dimension count needs +1. Find with:
```bash
grep -rn "continuous_bounds.*len\|param_names.*len\|45\|39" crates/ml/tests/dqn_action_collapse_fix_test.rs
```

Update `>= 39` to `>= 39` (already flexible from our previous fix) or verify no hardcoded count.

- [ ] **Step 7: Commit**

```bash
git add crates/ml/src/hyperopt/adapters/dqn.rs
git commit -m "feat: min_hold_bars in hyperopt search space [2, 20]

Integer parameter (rounded from continuous). Lets PSO discover
optimal hold duration for ES futures regime characteristics."
```

---

## Task 5: Action masking in `branching_action_select` (backtest path)

**Files:**
- Modify: `crates/ml/src/cuda_pipeline/epsilon_greedy_kernel.cu`
- Modify: `crates/ml/src/cuda_pipeline/gpu_action_selector.rs`

This is the secondary path — used by the backtest evaluator and tests, not the training loop.

- [ ] **Step 1: Add portfolio state parameters to `branching_action_select`**

In `epsilon_greedy_kernel.cu`, add after the `bonus_urgency` parameter:

```cuda
    const float* __restrict__ portfolio_state,  /* [batch_size, 20] or NULL */
    int min_hold_bars,
    float max_position
```

Note: `portfolio_state` can be NULL when called without hold enforcement (backward compat).

- [ ] **Step 2: Add hold masking logic**

After `int exposure;` (line 192), add:

```cuda
    /* Hold enforcement (when portfolio_state provided) */
    int in_hold = 0;
    int forced_exposure = -1;
    if (portfolio_state != NULL && min_hold_bars > 0) {
        float ht = portfolio_state[idx * 20 + 10];  /* hold_time */
        float pos = portfolio_state[idx * 20 + 0];   /* position */
        if (ht > 0.0f && ht < (float)min_hold_bars) {
            in_hold = 1;
            /* NOTE: branching_action_select currently hardcodes 5 exposure actions.
             * Use 4.0f = (5-1) for index mapping. If this kernel is later updated
             * to accept b0_size as a parameter, change to (b0_size-1). */
            float frac = pos / (max_position > 0.0f ? max_position : 1.0f);
            forced_exposure = (int)(roundf((frac + 1.0f) * 0.5f * 4.0f));
            if (forced_exposure < 0) forced_exposure = 0;
            if (forced_exposure >= 5) forced_exposure = 4;
        }
    }

    if (in_hold) {
        exposure = forced_exposure;
    } else {
```

Wrap existing exposure selection in `else { ... }`.

- [ ] **Step 3: Update Rust kernel launch**

In `gpu_action_selector.rs`, pass NULL for portfolio_state and 0 for min_hold_bars (backtest evaluator doesn't enforce holds during evaluation — greedy argmax only):

Add after the existing `.arg(...)` calls:
```rust
    .arg(&null_ptr)      // portfolio_state (NULL — no hold enforcement in eval)
    .arg(&0_i32)         // min_hold_bars (0 = disabled)
    .arg(&0.0_f32)       // max_position (unused when disabled)
```

Where `let null_ptr: u64 = 0;` (NULL pointer as device address).

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

Run: `SQLX_OFFLINE=true cargo check -p ml --lib`
Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
Expected: 887+ tests pass

- [ ] **Step 5: Commit**

```bash
git add crates/ml/src/cuda_pipeline/epsilon_greedy_kernel.cu crates/ml/src/cuda_pipeline/gpu_action_selector.rs
git commit -m "feat: action masking in branching_action_select (backtest/eval path)

Adds portfolio_state + min_hold_bars params (nullable for backward compat).
Backtest evaluator passes NULL (no hold enforcement during evaluation)."
```

---

## Task 6: Integration test — validate trade count reduction

**Files:**
- No new files — validation only

- [ ] **Step 1: Run full ml test suite**

Run: `SQLX_OFFLINE=true cargo test -p ml --lib`
Expected: 887+ pass, 0 fail

- [ ] **Step 2: Run ml-dqn test suite**

Run: `SQLX_OFFLINE=true cargo test -p ml-dqn --lib`
Expected: 359 pass, 0 fail

- [ ] **Step 3: Workspace compilation**

Run: `SQLX_OFFLINE=true cargo check --workspace`
Expected: clean

- [ ] **Step 4: PTX compilation test**

Run: `SQLX_OFFLINE=true cargo test -p ml --lib -- test_metrics_ptx`
Expected: pass (verifies CUDA kernels compile)

- [ ] **Step 5: Commit all changes and push**

```bash
git push origin main
```

---

## Execution Order

```
Task 1 (config) → Task 2 (Layer 2: kernel override) → Task 3 (Layer 1: action masking)
    → Task 4 (hyperopt) → Task 5 (backtest path) → Task 6 (integration test)
```

Tasks 1-3 are sequential (each builds on the previous). Task 4-5 are independent but ordered for clean git history. Task 6 validates everything.

**Expected outcome:** Trade count drops from ~620K to ~120K per epoch (with min_hold_bars=5). The model learns multi-bar swing trading instead of per-bar churning.
