foxhunt/docs/superpowers/specs/2026-05-31-eval-summary-trade-aggregation-design.md

Eval-Summary Trade Aggregation Fix — Design

Date: 2026-05-31 Status: Draft (pre-implementation) Branch target: ml-alpha-eval-summary-fix (off ml-alpha-regime-observer) Related: independent of Phase A (eval-diag emission); both ship in parallel Pearls / feedback referenced:

• feedback_no_partial_refactor
• feedback_single_source_of_truth_no_duplicates
• pearl_cluster_log_is_ground_truth

---

1. Motivation

eval_summary.json produced by alpha_rl_train is wrong at every scale. Cluster v11 (alpha-rl-8ll7j) reported pnl=+$61,513, max_dd=-$444,512, win_rate=0.217, n_trades=1024 for fold-1 eval. Local b=16 reproduction reported pnl=-$17,287, max_dd=$23,975, win_rate=0.414, n_trades=58. Both values are computed on a mixture of train-phase and eval-phase trades from a SINGLE backtest, not aggregated across all b_size accounts.

The cluster's improvement-vs-v10 narrative (v10 eval -$507k → v11 eval +$61k, falsely interpreted as a regime-observer success) is invalid for the same reason. Both numbers reflect whatever 1024 trades happened to occupy account-0's ring buffer at end-of-eval.

This bug has been silently corrupting every fold-1 eval result we've produced. Spotting it required Phase A pre-flight investigation; fixing it doesn't require Phase A but should ship alongside.

---

2. Root cause (verified locally)

2.1 The contaminated slice

alpha_rl_train.rs:743-1413:

let head_before_eval = sim.read_total_trade_count()?;       // (A) AGGREGATE across b_size
                                                            //     (sum of per-account head counters)
// ... eval loop runs ...
let all_records = sim.read_trade_records(0)?;               // (B) SINGLE backtest (#0), ≤ TRADE_LOG_CAP entries
let eval_records = if all_records.len() > head_before_usize {
    all_records[head_before_usize..].to_vec()               // (C) UNREACHABLE: per-account count ≤ 1024 ≤ b_size × per-account
} else {
    all_records                                             // (D) ALWAYS FIRES: returns ALL records,
                                                            //     including train-phase trades
};

2.2 Three independent flaws

1. Scale mismatch: head_before_eval is the aggregate trade count across all b_size backtests. all_records.len() is one backtest. Comparing them is wrong scales. At b=16, aggregate=600 but account-0=58 → branch (D) fires. At b=1024, aggregate=1,203,376 but account-0≤1024 → branch (D) fires.

2. Branch (D) ignores train/eval boundary: returns all 58 (local) or 1024 (cluster) records, INCLUDING pre-eval train trades. Eval_summary is computed on a mixed sample.

3. Single-account sampling: Even if (1) and (2) were fixed, reading only backtest 0 ignores the other b_size - 1 accounts. Eval performance should aggregate across the whole batch.

2.3 TRADE_LOG_CAP secondary issue

crates/ml-backtesting/src/lob/mod.rs:20:

pub const TRADE_LOG_CAP: usize = 1024;

At cluster scale (b=1024, 20k train + 5k eval, ~70 dones/step aggregate ≈ ~0.07 dones/account/step) each account accumulates ~1.4k–1.7k trades total. Capacity 1024 means heavy-trading accounts wrap during training, then wrap again during eval — even a correct slicing logic would be unable to isolate eval-only trades for those.

At local b=16, the per-account counts are well under 1024 (~37 train + 21 eval = 58 typical). No wrap. So a per-account aggregation fix is sufficient locally, but capacity may need bumping for cluster.

---

3. Goals

G1 — Aggregate across all backtests

eval_summary.json reflects all b_size accounts' eval-phase trades, not just account 0.

G2 — Correct train/eval slicing

Per-account head pointer snapshotted before eval; post-eval slice is records[head_before_per_b[i]..] for each i, handling wrap correctly.

G3 — Wrap detection + accurate accounting

When a backtest's pre-eval head was already past TRADE_LOG_CAP, log a warning + count of dropped pre-eval trades. Eval-phase trades that wrap are reported as a separate n_eval_trades_dropped field.

G4 — Capacity bump for cluster scale

TRADE_LOG_CAP raised from 1024 to a value that gives sufficient headroom at b=1024 production scale.

G5 — Non-goals

• Per-trade phase tagging (option C — adds 5 bytes/record × 1024 accounts × new_cap; deferred to a future spec if G3's wrap warnings prove frequent).
• Train-phase aggregate summary (current alpha_rl_train_summary.json is loss-only; trade-based metrics during train are not consumed by any downstream).

---

4. Design

4.1 New LobSimCuda methods

impl LobSimCuda {
    /// Read per-backtest `head` counter (length b_size). Returns the
    /// cumulative trade count for each backtest at call time. Used by
    /// alpha_rl_train to snapshot the pre-eval boundary per account.
    pub fn read_per_backtest_trade_counts(&self) -> Result<Vec<u32>>;

    /// Read all backtests' trade records. Returns Vec<Vec<TradeRecord>>
    /// of length b_size; entry i is the trade ring contents for backtest
    /// i (up to TRADE_LOG_CAP records, oldest dropped on wrap).
    ///
    /// One DtoH for trade_log_head_d + one DtoH for trade_log_d (same
    /// cost as read_trade_records(0) since the whole tensor was
    /// downloaded anyway — we just keep all the data instead of slicing
    /// to one account).
    pub fn read_trade_records_all(&self) -> Result<Vec<Vec<crate::order::TradeRecord>>>;
}

4.2 New alpha_rl_train.rs eval boundary handling

// At eval phase entry (before reset_session_state):
let head_before_per_b: Vec<u32> = sim.read_per_backtest_trade_counts()
    .context("snapshot per-backtest head pre-eval")?;
eprintln!(
    "── eval phase: {} steps on {} held-out files; pre-eval head per b_size={} accounts: \
     min={} max={} sum={} ──",
    cli.n_eval_steps, eval_files.len(), head_before_per_b.len(),
    head_before_per_b.iter().min().copied().unwrap_or(0),
    head_before_per_b.iter().max().copied().unwrap_or(0),
    head_before_per_b.iter().map(|&h| h as u64).sum::<u64>()
);

// ... eval loop runs ...

// Post-eval:
let all_per_b = sim.read_trade_records_all()
    .context("read all backtest trade records post-eval")?;
let head_after_per_b = sim.read_per_backtest_trade_counts()
    .context("snapshot per-backtest head post-eval")?;

let cap = crate::lob::TRADE_LOG_CAP as u32;
let mut eval_records: Vec<crate::order::TradeRecord> = Vec::new();
let mut n_eval_trades_dropped: u64 = 0;
let mut n_pre_eval_wrapped: u64 = 0;
let mut n_eval_trades_seen: u64 = 0;

for (b, records) in all_per_b.iter().enumerate() {
    let head_before = head_before_per_b[b];
    let head_after  = head_after_per_b[b];
    let eval_count_total = head_after - head_before;
    n_eval_trades_seen += eval_count_total as u64;

    // Records in the ring are the LAST min(head_after, cap) trades.
    // We want trades whose "index in the cumulative stream" lies in
    // [head_before, head_after). The ring's contents cover stream
    // indices [max(0, head_after - cap), head_after).
    let ring_start_stream_idx = head_after.saturating_sub(cap);

    if head_before < ring_start_stream_idx {
        // Pre-eval trades for this backtest already wrapped out before
        // eval started — purely informational, doesn't affect eval slicing.
        n_pre_eval_wrapped += (ring_start_stream_idx - head_before) as u64;
    }

    // Number of eval trades that wrapped out (lost):
    if eval_count_total > cap {
        n_eval_trades_dropped += (eval_count_total - cap) as u64;
    }

    // Slice the ring contents to eval-only:
    let eval_start_in_ring = head_before.saturating_sub(ring_start_stream_idx) as usize;
    let eval_slice = &records[eval_start_in_ring.min(records.len())..];
    eval_records.extend_from_slice(eval_slice);
}

if n_eval_trades_dropped > 0 {
    eprintln!(
        "warning: {} eval trades wrapped out across {} accounts (cap={}); summary based on \
         {} captured eval trades",
        n_eval_trades_dropped, all_per_b.len(), cap, eval_records.len()
    );
}
if n_pre_eval_wrapped > 0 {
    eprintln!(
        "info: {} pre-eval trades had wrapped before eval phase (no effect on eval summary)",
        n_pre_eval_wrapped
    );
}

// Compute summary on aggregated, eval-only records.
let mut pnl_curve = Vec::with_capacity(eval_records.len());
let mut cum = 0.0_f32;
for r in &eval_records {
    cum += (r.realised_pnl_usd_fp as f32) / 100.0;
    pnl_curve.push(cum);
}
let eval_summary = ml_backtesting::artifacts::compute_summary(&eval_records, &pnl_curve);

// Emit the existing fields plus two NEW fields:
//   n_eval_trades_dropped (u64) — total trades lost to ring wrap during eval
//   n_eval_trades_seen (u64)    — total eval trades that occurred (cumulative_done across b_size)
// These are written into eval_summary.json alongside the existing keys
// via a thin wrapper struct.

4.3 New eval_summary.json schema additions

Current:

{
  "n_trades": 1024,
  "total_pnl_usd": 61512.78,
  "profit_factor": 1.022,
  "sharpe_ann": 0.174,
  "max_drawdown_usd": 444512.31,
  "win_rate": 0.217
}

After fix:

{
  "n_trades": 348567,                          // ← aggregate across all backtests, eval-only
  "total_pnl_usd": 1234.56,
  "profit_factor": 1.085,
  "sharpe_ann": 0.42,
  "max_drawdown_usd": 12345.67,
  "win_rate": 0.39,
  "n_eval_trades_seen": 348700,                // NEW — true total eval-phase trade count
  "n_eval_trades_dropped": 133,                // NEW — 0 unless capacity insufficient
  "b_size": 1024,                              // NEW — context for downstream interpretation
  "n_pre_eval_trades_wrapped": 5821            // NEW — diagnostic, doesn't affect eval metrics
}

compute_summary already consumes &[TradeRecord] and produces n_trades, total_pnl_usd, ... — feeding it the aggregated eval_records Just Works for those fields. The four new fields are added by the alpha_rl_train wrapper.

4.4 TRADE_LOG_CAP bump

Cluster b=1024, fold-1 eval, 5000 steps observed:

• Total eval dones aggregate ≈ 350,000
• Per account average: ~342
• Per account max (heavy traders): likely 2–3× mean ≈ 700–1000

Setting TRADE_LOG_CAP = 4096 gives:

• 4× headroom over observed mean
• Memory cost at b=1024: 1024 × 4096 × 40 B = 167 MB GPU (was 41 MB) — fits comfortably on L40S (48 GB) and H100 (80 GB)
• For train-phase carryover at b=1024 with ~1175 dones/account, no wrap during normal training either

For local smoke at b=16: memory cost = 16 × 4096 × 40 B = 2.6 MB — negligible.

// crates/ml-backtesting/src/lob/mod.rs
pub const TRADE_LOG_CAP: usize = 4096;  // was 1024 (2026-05-31: bumped to prevent eval wrap)

4.5 Backward compatibility

The two existing methods (read_trade_records(b), read_total_trade_count()) stay as-is — they may be used elsewhere (unit tests, debug paths). The new methods are additive.

The existing alpha_rl_train.rs:1402-1413 block is REPLACED by the new aggregation block; no need for a feature flag.

---

5. Validation criteria

V1 — Local smoke math correctness

At b=16, 1k train + 250 eval, fold 1:

• Pre-eval log line shows head_before_per_b: min=X max=Y sum=Z (Z should match the previous head=Z log)
• Post-eval n_eval_trades_seen ≈ eval_dones_step_diag_total - 0 (eval_dones reported per-step in the JSONL with Phase A)
• n_eval_trades_dropped = 0 (no wrap at b=16)
• n_trades in eval_summary ≈ n_eval_trades_seen (no wrap → equal)

V2 — Train/eval contamination test

At b=16, 1k train + 0 eval:

• No eval phase runs
• eval_summary.json NOT written (existing gate: n_eval_steps > 0)

At b=16, 0 train + 250 eval (synthetic edge case):

• Pre-eval head_before_per_b all zeros
• eval_summary.n_trades = sum of (post-eval head_per_b)
• No contamination possible (no train phase)

V3 — Cluster scale validation

At b=1024, 20k train + 5k eval, fold 1:

• n_eval_trades_dropped should be 0 with TRADE_LOG_CAP=4096
• n_trades should be O(350,000), not O(1,024)
• total_pnl_usd magnitude makes sense (b=1024 × per-account small pnl; should be in $100k-$1M range)
• win_rate should NOT be the suspiciously-low 0.217 we observed (which was contaminated)

V4 — TRADE_LOG_CAP stress test

Run b=1024 with intentionally heavy trading (force flat-frequency policy via seed) and 10k eval steps. Verify n_eval_trades_dropped > 0 triggers the warning correctly and the summary still produces sensible numbers (just on a subset).

---

6. Implementation phases

Phase 1 — Lobsim API additions (~1 hour)

1. Add read_per_backtest_trade_counts() to LobSimCuda — single DtoH of trade_log_head_d into Vec<u32>
2. Add read_trade_records_all() — same memcpy as read_trade_records(0) but split into per-backtest slices
3. Unit test: push synthetic trades, verify both methods return expected counts/records

Phase 2 — TRADE_LOG_CAP bump (~5 min)

1. Change pub const TRADE_LOG_CAP: usize = 1024; → = 4096;
2. Verify no asserts/tests hardcode 1024 (grep 1024 in lob module)
3. Run existing lobsim unit tests

Phase 3 — alpha_rl_train aggregation (~30 min)

1. Replace the existing slicing block (lines 1402-1413) with the aggregation logic in §4.2
2. Add the four new fields to a typed struct that augments compute_summary's output
3. Update the eprintln messages

Phase 4 — Local validation (V1, V2) (~15 min)

1. Run b=16, 1k+250 eval smoke
2. Assert n_trades > 50 (was 58; should now be sum across 16 backtests ≈ 16 × eval_avg ≈ 200-400)
3. Verify n_eval_trades_dropped = 0
4. Visually inspect that win_rate / pf / sharpe values look more reasonable

Phase 5 — Push for cluster validation (V3) (~minutes)

1. Push
2. Submit alpha-rl run with same config as v11
3. Compare eval_summary.json to v11's — n_trades should be ~340× larger

---

7. Open decisions

#	Question	Recommendation
1	TRADE_LOG_CAP value	4096 — 4× cluster mean, fits memory budget
2	Compute_summary signature change for new fields	No — wrap output in a thin struct in alpha_rl_train.rs that adds the four new fields. compute_summary stays untouched.
3	Pre-eval head snapshot stream sync	Yes — must sync before reading head_d to avoid stale values mid-step (universal save-rule applies)
4	Bump train-phase artifact too?	No — alpha_rl_train_summary.json only has losses, no trade metrics. Keep scope tight.
5	Deprecate read_trade_records(b) + read_total_trade_count()	No — keep for unit tests and future debugging. Only the alpha_rl_train call site changes.
6	Add per-fold artifact directory cleanup	No — out of scope.

---

8. Interaction with Phase A (eval-diag emission)

Phase A and this fix are independent:

• Phase A: emits per-step JSONL during eval phase
• This fix: corrects the aggregate eval_summary.json

After both ship, downstream consumers can choose:

• Per-step: use eval_diag.jsonl for trajectory analysis (when did drawdown happen, etc.)
• Aggregate: use corrected eval_summary.json for cross-run comparison (G8 gate, profit factor)

Neither replaces the other.

---

9. Done means

• Phase 1-4 implemented and locally validated
• One cluster run produces an eval_summary.json with all new fields populated and n_trades in the hundreds of thousands
• Pearl saved: pearl_eval_summary_aggregation_bug.md documenting the bug class
• MEMORY.md updated

---

10. Risks

• Stream sync: forgetting to sim.stream.synchronize() before read_per_backtest_trade_counts() could return stale head values mid-step. Mitigation: every read call synchronizes internally (existing pattern in read_trade_records).
• Memory cost spike: TRADE_LOG_CAP 1024→4096 = 4× memory. Already accounted for; fits budget. Mitigation: if it doesn't fit, fall back to 2048 + accept higher wrap rate.
• TradeRecord field interpretation: realised_pnl_usd_fp is fixed-point. Existing code divides by 100; preserve that.
• Backward compat for old eval_summary.json consumers: Argo aggregator (per memory pearl_single_window_oos_is_not_oos) reads profit_factor and total_pnl_usd. These keys are preserved. New fields are additive.