spec(sp11): amend post-A2 audit findings — 6 architectural gaps resolved

The B1 implementer's audit during atomic-consumer-migration surfaced 6 real gaps in the original spec that the existing code reveals (per feedback_trust_code_not_docs). Spec amended in place; B1 was correctly BLOCKED until these resolved. No code change required for amendment; A2's already-landed renormalization changes from Σ=1 to mean=1 in B0 (small follow-up commit). §3.4.3 mean-normalization (NEW): A2's Σ=1 renormalization causes 6× reward-magnitude collapse on mutually-exclusive components (popart / micro / opp_cost paths). Amended to mean(weights) = 1, i.e., Σ = N = 6. Each weight ∈ [floor, MAX_WEIGHT=3.0]. Default uniform = 1.0 each. Preserves pre-SP11 absolute scale on average. Smoke pass criterion in §9.1 updated accordingly. §3.4.4 universal post-composition modifiers (NEW): drawdown, capital- floor, inventory, churn, conviction-scale, cf-flip are RISK CONSTRAINTS or STRUCTURAL OPERATORS, not weighted reward components. They apply post-composition, unweighted. Spec §3.2's 6 weights govern WHICH learning signals matter; modifiers govern what the model must always attend to regardless of the controller. §3.5 amendment: REWARD_CF_WEIGHT_INDEX scope clarified — the cf_weight=0.3f at mse_loss_kernel.cu:318 + c51_loss_kernel.cu:789 are STRUCTURAL Q-blend weights for Hold-sample magnitude branch gradient, NOT reward weights. The original spec's directive to migrate them was an audit error. They stay untouched. REWARD_CF_WEIGHT_INDEX applies ONLY to the cf-reward path inside experience_env_step. §3.5.3 structural decomposition (NEW): explicit per-component locals (r_popart/r_cf/r_trail/r_micro/r_opp_cost/r_bonus) replace the 8+ inline accumulation sites in experience_env_step. Final composition is Σ w_i × r_i with weighted-Σ + post-composition modifiers per §3.4.4. §3.5.4 trail reward extraction (NEW): pre-SP11 trail-fire P&L flows through segment_complete = popart, leaving REWARD_TRAIL_WEIGHT_INDEX structurally inert. Architectural change: segregate trail-fire bars from voluntary-exit bars in segment_complete P&L credit so r_trail has real signal and the controller can weigh forced-exit vs voluntary-exit P&L differently. §3.5.5 replay-time curiosity audit gate (NEW): B1c (curiosity wiring) depends on a verifiable claim that trainer.rewards_buf is the SINGLE read site for replay rewards across mse_loss/c51_loss/IQN/CQL/Bellman. Layer C audit task added as pre-requisite to B1c. §8 Layer B split into B0/B1a/B1b/B1c (was 1 atomic ~750 LOC commit): - B0 (~30 LOC): A2 controller renormalization Σ=1 → mean=1 + MAX_WEIGHT cap + 3 unit-test assertion updates. - B1a (~150 LOC): saboteur GPU multiplication + SimHash state_stride parameter. Small/safe migrations. - B1b (~500 LOC, atomic): structural reward-composition refactor per §3.5.3 + r_trail extraction per §3.5.4 + universal modifiers per §3.4.4. - B1c (~150 LOC, post-Layer-C-audit): replay-time curiosity wiring. Splitting them respects feedback_no_partial_refactor (each commit is atomic for its own contract change) without forcing a 1500-LOC atomic blast. Spec line count: 565 → 804 (+239 lines). MAX_WEIGHT added to §3.4.2 Invariant-1 anchor list. §7 component-starvation note + §9.3 fix-forward note updated to reflect mean=1.
2026-05-04 08:29:29 +02:00
parent e0d3abd9d2
commit 7ddaf9c515
1 changed files with 271 additions and 29 deletions
--- a/docs/superpowers/specs/2026-05-04-sp11-reward-as-controlled-subsystem.md
+++ b/docs/superpowers/specs/2026-05-04-sp11-reward-as-controlled-subsystem.md
@@ -197,11 +197,14 @@ for (int c = 0; c < 6; c++) {
    blends[c] = w;
    blend_sum += w;
 }
-// Renormalize so Σweights = 1 exactly. EPS_DIV guards the (impossible)
-// all-floor-zero case.
-float inv_sum = 1.0f / fmaxf(blend_sum, EPS_DIV);
+// Renormalize so mean(weights) = 1 (Σ = N = 6). EPS_DIV guards the
+// (impossible) all-floor-zero case. **Amended 2026-05-04 — see §3.4.3
+// for rationale (the original A2-landed code used Σ = 1, which collapses
+// reward magnitude 6× on mutually-exclusive components).**
+const float N_components = 6.0f;
+float scale = N_components / fmaxf(blend_sum, EPS_DIV);
 for (int c = 0; c < 6; c++) {
-    scratch[scratch_weight_base + c] = blends[c] * inv_sum;
+    scratch[scratch_weight_base + c] = fminf(MAX_WEIGHT, blends[c] * scale);
 }

 // Curiosity bound = fraction of recent PnL magnitude (signal-relative)
@@ -251,6 +254,7 @@ and remain valid across regime shifts:
 |---|---|---|
 | `EPS_DIV` | `1e-6f` | divide-by-zero protection |
 | `WEIGHT_HARD_FLOOR` | `0.01f` | absolute weight floor — prevents zero |
+| `MAX_WEIGHT` | `3.0f` | per-component upper cap (mean=1; cap = 3× over-weighted) |
 | `SABOTEUR_MIN` / `MAX` | `0.5f` / `2.0f` | curriculum rate limiter |
 | `CURIOSITY_PERMANENT_FRACTION` | `0.2f` | exploration floor as fraction of `curiosity_bound` |
 | `CURIOSITY_BOUND_FRACTION` | `0.3f` | curiosity ceiling as fraction of PnL EMA |
@@ -261,13 +265,106 @@ The thresholds that classify regime ("improving / stagnant / regressing")
 are entirely encoded in the Z-score and sigmoid — no hardcoded threshold
 survives at the regime layer.

+#### 3.4.3 Mean-normalization (amendment 2026-05-04 post-A2 audit)
+
+**A2-landed code uses `Σweights = 1`.** Implementer audit during B1 surfaced
+that `experience_env_step` reward composition is *mutually-exclusive
+inline accumulation* (popart / micro / opp_cost are alternative paths;
+exactly one fires per bar), so `Σ w_i × r_i` reduces to `single_weight ×
+single_value`. With Σ=1 across 6 components, the average per-bar weight
+is ~1/6 → 6× collapse of reward magnitude vs pre-SP11. PopArt EMA tracks
+but adapts on a multi-epoch timeconstant, interacting with C51 v_range /
+IQN τ adaptation and risking cold-start training pathology.
+
+**Amendment:** weights are normalized to **mean = 1.0** (i.e., `Σ = N
+where N = 6`), not `Σ = 1`. Each weight ∈ [floor, MAX_WEIGHT] where
+`MAX_WEIGHT = 3.0` (Invariant-1 cap preventing any single weight
+dominating ≥ 50% of the 6-component budget). Default uniform = 1.0
+each. The controller's renormalization step in §3.4 changes from:
+
+```cuda
+inv_sum = 1.0 / max(blend_sum, EPS_DIV);
+scratch[c] = blends[c] * inv_sum;          // Σ = 1
+```
+
+to:
+
+```cuda
+scale = (float)N / max(blend_sum, EPS_DIV);
+scratch[c] = fminf(MAX_WEIGHT, blends[c] * scale);   // mean = 1
+```
+
+This preserves pre-SP11 absolute scale on average while letting the
+controller redistribute share among components. PopArt's `popart` slot
+weight (default 1.0) acts as a multiplicative around 1.0× of pre-SP11
+trade-PnL, not a 1/6 attenuation.
+
+§9.1 smoke pass criterion changes from `Σweights ≈ 1.0 ± 1e-3` to
+`mean(weights) ≈ 1.0 ± 1e-3` (i.e., Σ ≈ 6.0 ± 6e-3 with N=6 components).
+
+#### 3.4.4 Universal post-composition modifiers (amendment 2026-05-04)
+
+Six modifiers in `experience_env_step` apply AFTER the if/else reward
+composition: drawdown penalty, capital-floor cap, inventory penalty,
+churn penalty, conviction-scale multiplier, G7 counterfactual flip.
+**These are RISK CONSTRAINTS or STRUCTURAL OPERATORS, not reward
+components** — they don't get controller weights. Concretely:
+
+- **Drawdown / inventory / churn penalties** are unconditional risk
+  guards that fire on every bar matching their condition. The whole
+  point is that the agent CANNOT learn to weigh them away. They apply
+  post-composition, unweighted.
+- **Capital-floor cap** is a hard clamp `r = max(-10, -5 - 5×risk_taken)`
+  on capital breach — a structural safety bound, not a weighted
+  contribution.
+- **Plan conviction scale** is a multiplier `r *= conviction` driven by
+  the plan head's confidence — modulates the existing reward, not a
+  separate component to weight.
+- **G7 cf-flip** is `r *= -1` on counterfactual samples — a structural
+  operator on the cf path, not a learning weight.
+
+Composition with universal modifiers:
+
+```
+r_weighted = Σ w_i × r_i                     // SP11 controller-weighted
+r_total    = clamp_capital(r_weighted)       // hard clamp
+r_total   -= drawdown_penalty                // unconditional risk guard
+r_total   -= inventory_penalty               // unconditional risk guard
+r_total   -= churn_penalty                   // unconditional risk guard
+r_total   *= conviction_scale                // plan modulator
+r_total   *= cf_flip                         // structural operator
+out_rewards[out_off] = r_total
+```
+
+The controller's 6 weights govern WHICH learning signals matter
+(`Σ w_i × r_i`). The post-composition modifiers govern what the model
+must always pay attention to regardless of the controller. Both are
+correct separately; no controller weights for the modifiers.
+
 ### 3.5 Consumer migrations

-**Reward shaping kernels** (consume per-component weights from ISV):
- `mse_loss_kernel.cu:318` — replace `cf_weight = 0.3f` with
-  `isv[REWARD_CF_WEIGHT_INDEX]`
- `c51_loss_kernel.cu:789` — same migration
- Other reward shaping multipliers identified during implementation audit
+**`REWARD_CF_WEIGHT_INDEX` scope clarification (amendment 2026-05-04):**
+The `cf_weight = 0.3f` constants at `mse_loss_kernel.cu:318` and
+`c51_loss_kernel.cu:789` are **structural Q-distribution mixing weights
+for Hold-sample magnitude branch gradient**, NOT reward-shaping weights.
+They happen to share the variable name with the cf-reward path but are a
+DIFFERENT signal entirely. The original draft of this spec listed them
+for migration; that was an audit error. Leave both untouched.
+
+`REWARD_CF_WEIGHT_INDEX` applies ONLY to the cf-reward path inside
+`experience_env_step` — the counterfactual-sample reward written to
+`out_rewards[cf_off]` and consumed by the loss kernels via the replay
+buffer's cf tuples. The structural-refactor composition `Σ w_i × r_i`
+applies `w_cf × r_cf` for cf samples; on-policy samples use
+`w_popart × r_popart` (or whichever path fired) without crossing the cf
+weight at all.
+
+**Reward composition site** (the single migration target — see §3.5.3
+for the structural decomposition):
+- `experience_env_step` reward composition: 8+ inline accumulation sites
+  decomposed into per-component locals, then composed via the weighted
+  Σ formula in §3.4.4 (with `mean(weights) = 1` normalization per §3.4.3
+  and post-composition risk modifiers per §3.4.4).

 **Curiosity bonus** (NEW reward dimension, applied at *loss-compute* time
 not at experience-collect time — see §3.5.1):
@@ -279,6 +376,110 @@ not at experience-collect time — see §3.5.1):
  * novelty_signal(state, action)`
 - Bounded above by `CURIOSITY_BOUND_INDEX` in the producer kernel.

+#### 3.5.3 Structural decomposition of `experience_env_step` reward composition (amendment 2026-05-04)
+
+The pre-SP11 `experience_env_step` accumulates reward in-place across
+8+ inline sites (search audit identified `:2587` trade-PnL base, `:2845`
+micro overwrite, `:2963` Flat opp-cost, `:2646/:2667/:2718/:3011/:3061`
+bonus accumulations, `:3400` cf_reward — line numbers as of 2026-05-04
+audit; verify at implementation time). The migration replaces this with
+explicit per-component locals + a final weighted sum:
+
+```cuda
+float r_popart   = 0.0f;   // trade-PnL on segment-complete bars
+float r_cf       = 0.0f;   // counterfactual-sample reward
+float r_trail    = 0.0f;   // trail-fire P&L (§3.5.4)
+float r_micro    = 0.0f;   // OFI-driven micro-reward (mid-trade)
+float r_opp_cost = 0.0f;   // Flat opportunity cost
+float r_bonus    = 0.0f;   // B/C/D bonuses summed
+
+// ... existing per-bar logic populates ONE OR MORE of the locals ...
+
+const float w_popart   = isv[REWARD_POPART_WEIGHT_INDEX];
+const float w_cf       = isv[REWARD_CF_WEIGHT_INDEX];
+const float w_trail    = isv[REWARD_TRAIL_WEIGHT_INDEX];
+const float w_micro    = isv[REWARD_MICRO_WEIGHT_INDEX];
+const float w_opp_cost = isv[REWARD_OPP_COST_WEIGHT_INDEX];
+const float w_bonus    = isv[REWARD_BONUS_WEIGHT_INDEX];
+
+float r_weighted = w_popart   * r_popart
+                 + w_cf       * r_cf
+                 + w_trail    * r_trail
+                 + w_micro    * r_micro
+                 + w_opp_cost * r_opp_cost
+                 + w_bonus    * r_bonus;
+
+// Universal post-composition modifiers (§3.4.4):
+float r_total = clamp_capital(r_weighted);
+r_total -= drawdown_penalty;
+r_total -= inventory_penalty;
+r_total -= churn_penalty;
+r_total *= conviction_scale;
+r_total *= cf_flip;
+
+total_reward_per_sample[out_off] = r_total;
+```
+
+**Mutual-exclusivity preservation:** popart / micro / opp_cost are
+mutually exclusive paths in the if/else chain. The migration writes 0
+to the locals that don't fire and the relevant local to the active
+component. The weighted sum produces `w_active × r_active` per bar (one
+multiply, not 6), which with `mean(w) = 1` (§3.4.3) preserves the
+pre-SP11 absolute scale.
+
+#### 3.5.4 Trail reward extraction (amendment 2026-05-04)
+
+In pre-SP11 code, trail-fire P&L flows through `segment_complete = true`
+because `exiting_trade = trail_triggered ? 1 : wants_exit` — both
+voluntary-exit and trail-forced-exit go through the same popart-credit
+path. So `r_trail` would be structurally inert (always 0) under a naive
+decomposition, leaving `REWARD_TRAIL_WEIGHT_INDEX` as a dead controller
+output.
+
+**Architectural change:** segregate trail-fire bars from voluntary-exit
+bars in `segment_complete`'s P&L credit:
+
+```cuda
+if (segment_complete) {
+    const float pnl = computed_segment_pnl;
+    if (trail_triggered) {
+        r_trail = pnl;     // forced-exit P&L → trail signal
+    } else {
+        r_popart = pnl;    // voluntary-exit P&L → popart signal
+    }
+    // ... existing segment_complete bonus accumulations ...
+}
+```
+
+This gives the controller meaningful signal on `w_trail` to weigh
+forced-exit P&L differently from voluntary-exit P&L (e.g., upweight
+forced-exit when trail-stop fires too aggressively, downweight when
+trail-stop is too loose). The `pearl_trail_fire_pre_vs_action_mag`
+already documents that trail filtering uses prior-position magnitude;
+this extension lets the controller learn whether the trail-stop is
+producing too much/too little forced-exit signal.
+
+#### 3.5.5 Replay-time curiosity audit gate (amendment 2026-05-04)
+
+The B1c (curiosity wiring) commit depends on a verifiable claim: the
+trainer's `rewards_buf` is the SINGLE read site for replay rewards
+across ALL downstream consumers (mse_loss, c51_loss, IQN target, CQL,
+Bellman target). If any consumer reads from a different buffer (e.g., a
+target-specific reward buffer populated separately), post-gather
+curiosity-add doesn't propagate to that consumer, creating a partial
+migration violating `feedback_no_partial_refactor`.
+
+**Layer C audit task (precedes B1c commit):**
+1. `grep -rn "rewards_buf\|rewards_dev\|reward_target" crates/ml/src/cuda_pipeline/`
+2. Trace each non-write read; document the consumer + which buffer it
+   reads.
+3. Verdict matrix: { consumer → which buffer it reads }. Pass criterion:
+   every consumer reads from `trainer.rewards_buf` (the post-gather
+   buffer that B1c augments with curiosity). If any consumer reads
+   elsewhere, B1c either (a) extends to that buffer, or (b) the spec
+   is amended to acknowledge that consumer is intentionally
+   curiosity-free.
+
 #### 3.5.1 Why recompute-at-replay (not store-at-collect)

 If we store curiosity-augmented r in replay, old tuples retain the
@@ -407,9 +608,11 @@ Plus modifications to existing kernels:
  curiosity always (overfitting prevention).
 - **Component starvation guard**: Per-component weight reweighting could in
  principle starve a contributor. Mitigated by adaptive floor
-  `≥ 0.5 × min_grad_ratio` and post-floor renormalization to Σ=1, ensuring
-  every component keeps a non-trivial budget without inflating total
-  reward magnitude (which would contaminate PopArt).
+  `≥ 0.5 × min_grad_ratio` and post-floor mean-normalization to
+  `mean(weights) = 1` (§3.4.3), ensuring every component keeps a
+  non-trivial budget without collapsing reward magnitude (mean=1 preserves
+  pre-SP11 absolute scale on average; `Σ=1` would cause 6× collapse on
+  mutually-exclusive components).
 - **Saboteur intensity adapts both directions**: When val sharpe is
  improving, saboteur intensity rises to keep the policy under realistic
  adversarial load (you don't want a model that only works when execution
@@ -462,23 +665,61 @@ validation — there is no gating preflight that pauses or defers work.
 - Build.rs entry, launcher, training_loop wire-up
 - HEALTH_DIAG `sp11_reward` line emits 10 outputs + `improvement_z`

-### Layer B — atomic consumer migration (1 commit, ~750 LOC)
+### Layer B — split into B0/B1a/B1b/B1c (amendment 2026-05-04 post-A2 audit)

-`feedback_no_partial_refactor`: every consumer of the migrated
-contract migrates in one commit. The trainer is on the new controller
-end of this commit.
+`feedback_no_partial_refactor` requires every consumer of a CHANGED
+CONTRACT to migrate together. Layer B is decomposed into four commits
+because the original "single atomic Layer B" mixed three independent
+contract changes (controller weight semantics, saboteur multiplier,
+replay-time curiosity) — splitting them lets each contract migrate
+atomically without forcing a 1500-LOC blast in one commit, and lets
+B0 (controller-renorm change) precede B1b (which depends on the new
+mean=1 semantics).

- `mse_loss_kernel.cu:318` — `cf_weight = isv[REWARD_CF_WEIGHT_INDEX]`
- `c51_loss_kernel.cu:789` — same migration
- Full audit + migrate every hardcoded reward-shaping constant in
-  `cuda_pipeline/*.cu` (expected ~3-5 additional sites; implementer
-  enumerates and migrates all of them)
- `gpu_experience_collector.rs:3328` — `let ps = saboteur_perturbation_scale
-  * isv[SABOTEUR_INTENSITY_MULT_INDEX]`
- Replay sampling kernel: curiosity bonus added **at replay time**
-  (§3.5.1) reading `isv[CURIOSITY_PRESSURE_INDEX]` × `novelty_signal`
- Reward composition reads all 6 component weights from ISV
- Stale doc-string sweep across migrated sites
+**B0 — A2 controller renormalization amendment (~30 LOC)**
+Per §3.4.3, change the A2-landed controller from `Σweights = 1` to
+`mean(weights) = 1` (i.e., `Σ = N = 6`). Modify
+`reward_subsystem_controller_kernel.cu` renormalization step to multiply
+by `N` after the divide. Add `MAX_WEIGHT = 3.0` Invariant-1 cap. Update
+the 3 A2 unit tests' assertions (mean-around-1 instead of sum-to-1).
+Smoke verifies HEALTH_DIAG `sp11_reward` shows weights ≈ 1.0 each on
+uniform-grad-ratio inputs.
+
+**B1a — small/safe migrations (~150 LOC)**
+- Saboteur GPU multiplication: `saboteur_generate_params` kernel reads
+  `isv[SABOTEUR_INTENSITY_MULT_INDEX]` on-device, computes
+  `effective_scale = base × mult`. `gpu_experience_collector.rs:3328`
+  passes ISV pointer + slot index to the kernel; no host-side multiply.
+- SimHash kernel `state_stride` parameter added (states_buf is
+  STATE_DIM_PADDED=128, but kernel needs to read 42 contiguous floats
+  per sample). Kernel signature gains `int state_stride`; call sites
+  pass `STATE_DIM_PADDED`.
+
+**B1b — structural reward-composition refactor (~500 LOC, atomic)**
+- `experience_env_step` reward decomposition per §3.5.3 (8+ inline sites
+  → per-component locals → weighted Σ).
+- §3.5.4 r_trail extraction: segregate trail-fire bars from voluntary-
+  exit bars in `segment_complete` P&L credit.
+- §3.4.4 universal post-composition modifiers: drawdown/inventory/
+  churn/conviction-scale/cf-flip applied AFTER `Σ w_i × r_i`,
+  unweighted.
+- `mse_loss_kernel.cu:318` and `c51_loss_kernel.cu:789` `cf_weight =
+  0.3f` constants UNTOUCHED per §3.5 amendment (they're structural
+  Q-blend, not reward weights).
+- Stale doc-string sweep across modified sites.
+- Smoke verifies HEALTH_DIAG `sp11_reward` weights drift across epochs
+  AND popart EMA tracks ≈ pre-SP11 magnitude (mean=1 normalization
+  preserves scale).
+
+**B1c — replay-time curiosity (post-Layer-C-audit, ~150 LOC)**
+- Pre-requisite: Layer C audit per §3.5.5 confirms `trainer.rewards_buf`
+  is the single read site for all replay-reward consumers.
+- Trainer launches lookup → apply_curiosity_bonus → update sequence
+  immediately after `upload_batch_gpu` returns base rewards.
+- New kernel `apply_curiosity_bonus_kernel.cu` (in-place
+  `r += curiosity_p × novelty(state, action)` on `trainer.rewards_buf`).
+- Pre-existing `gather_f32_scalar` in `ml-dqn` left untouched (semantic
+  equivalence preserved without cross-crate refactor).

 ### Layer C — validation + close-out

@@ -500,7 +741,8 @@ T10 ≈ 4 hr).

 Hard-pass criteria (all must hold):
 - HEALTH_DIAG `sp11_reward` line emits every epoch with all weights in
-  [adaptive_floor, 1.0] and Σweights ≈ 1.0 ± 1e-3 (renormalization works)
+  [adaptive_floor, MAX_WEIGHT=3.0] and `mean(weights) ≈ 1.0 ± 1e-3`
+  (i.e., `Σ ≈ 6.0 ± 6e-3` with N=6 components, per §3.4.3)
 - `improvement_z` is non-zero by ep2 (canary populates after Pearl A
  bootstrap on ep1's first delta)
 - `curiosity_pressure ≥ 0.2 × curiosity_bound` always (permanent floor)
@@ -545,7 +787,7 @@ Fix-forward, not roll-back. Failure modes and responses:
  shaping multipliers found during Layer B audit
 - z-canary stuck at 0: variance EMA is mis-tracking; review Pearl A
  bootstrap on `VAL_SHARPE_VAR_EMA`
- Component weights collapse to floor: renormalization or floor
+- Component weights collapse to floor: mean-normalization or floor
  computation bug; review §3.4 implementation
 - Curiosity dominates loss (`r_total / r_base > 1.0` median):
  CURIOSITY_BOUND_FRACTION too high; this is the only Invariant-1