Files
foxhunt/scripts/pre-commit-hook.sh
jgrusewski ce1e13519b fix(rl): mapped-pinned for all R7d/R8 CPU↔GPU paths + diag JSONL + guard
Two concerns in one commit since they're entangled:

## 1. feedback_no_htod_htoh_only_mapped_pinned violations

R7d (PER push/sample) + R8 (CLI binary) + the new per-step diag dump
shipped with 8 raw `stream.memcpy_htod` / `stream.memcpy_dtoh` calls.
The rule is explicit: "mapped-pinned only for CPU↔GPU; tests not
exempt." A raw `stream.memcpy_*` on a regular `&[T]` / `&mut [T]` is
NOT mapped-pinned — the source/dest slice isn't page-locked, so the
CUDA driver does an internal blocking HtoD/DtoH that stalls the
stream.

Refactored all 8 violations to use the mapped-pinned + DtoD pattern
(cuMemHostAlloc DEVICEMAP — host writes via `host_ptr`, kernel reads
`dev_ptr`, DtoD between them via `cudarc::driver::result::memcpy_dtod_async`).
New shared helpers in `trainer/integrated.rs`:

  * `read_slice_i32_d` — DtoH for `i32` device buffers via
    `MappedI32Buffer` staging. Counterpart to the existing
    `read_slice_d` (f32 version).
  * `write_slice_f32_d` — CPU→GPU upload for `f32` via
    `MappedF32Buffer.write_from_slice` + DtoD into destination.
  * `write_slice_i32_d` — CPU→GPU upload for `i32` via
    `MappedI32Buffer.host_slice_mut().copy_from_slice` + DtoD.

`pub fn` wrappers (`read_slice_*_d_pub`) expose the f32/i32 helpers
to the CLI binary so the per-step diag DtoH uses the same canonical
pattern.

Call-site refactors:

  * `push_to_replay`: 4× `stream.memcpy_dtoh` → `read_slice_*_d`.
  * `sample_and_gather`: 3× `stream.memcpy_htod` → `write_slice_*_d`.
  * `step_with_lobsim` pre-PER ISV refresh: raw `memcpy_dtoh` →
    `read_slice_d` (424 floats per step).
  * `step_with_lobsim` post-Q PER priority TD readback: raw
    `memcpy_dtoh` → `read_slice_d` (b_size floats per step).
  * `step_synthetic` ISV mirror refresh: raw `memcpy_dtoh` →
    `read_slice_d` (pre-existing pre-R9 violation; fixed in the
    same commit since it's the same pattern in the same file).
  * Init-time (one-shot) `prng_state` upload: raw `memcpy_htod` →
    inline mapped-pinned DtoD (custom because cast through i32 for
    the u32 buffer).
  * Init-time (one-shot) `atom_supports` upload: raw `memcpy_htod`
    → `write_slice_f32_d`.
  * `examples/alpha_rl_train.rs` per-step diag DtoH (3 calls) →
    `read_slice_*_d_pub`.

## 2. Pre-commit guard gap — diff-aware HtoD/DtoH check

The existing GPU hot-path guard (`scripts/gpu-hotpath-guard.sh`)
EXPLICITLY skips memcpy_htod/dtoh on the assumption that such calls
only appear in `cuda_pipeline/` (where mapped-pinned is the
convention). That assumption was falsified by R7d/R8 — the guard
shipped 8 violations green.

Added `check_no_raw_htod_dtoh` to `scripts/pre-commit-hook.sh` (the
real file behind the `.git/hooks/pre-commit` symlink). The check is
DIFF-AWARE: it greps only the `+` lines of `git diff --cached -U0`,
so pre-existing violations elsewhere (143 sites across the codebase)
don't block commits touching unrelated files. NEW additions of
`\.memcpy_(htod|dtoh)\(` are flagged with a clear error pointing at
the mapped-pinned alternative. Suppress per-line with `// gpu-ok:
<reason>` (same convention as the existing guards).

Pre-existing violations in `ml-alpha/src/aux_heads.rs`,
`mamba2_block.rs`, `cfc/`, `data/`, etc. are a separate cleanup —
not blocked by this commit's check because the diff-aware filter
ignores anything that was already on `HEAD~1`.

## Verified gates (post-fix, local sm_86)

  G1  isv_bootstrap                 unchanged
  G3  controllers_emit              unchanged
  G4  target_soft_update            unchanged
  G6  r7d_per_wiring                unchanged (PER round-trips all
                                     mapped-pinned now)
  R3  ema/advantage (3 tests)       unchanged
  R4  action kernels (3 tests)      unchanged
  end integrated_trainer_smoke      unchanged

Mapped-pinned is semantically equivalent to raw memcpy_htod/dtoh —
just routed through page-locked staging so the driver doesn't have
to do its own internal pinning. Behaviour identical; the cost shifts
from "driver hidden HtoD per call" to "mapped-pinned alloc + DtoD
per call." For the smoke (b_size=1, 1000 steps), the cost difference
is in the microseconds.

## Per-step diag JSONL (separate concern, same commit)

Added `--diag-jsonl <PATH>` flag to `alpha_rl_train.rs` (default:
`<out>/diag.jsonl`). After each `step_with_lobsim`, writes one JSON
record capturing:

  * step number, elapsed wall time
  * all 5 head losses + λs
  * all 7 RL controller outputs (γ τ ε coef n_roll per_α scale)
  * all 5 per-head learning rates (lr_bce/q/pi/v/aux)
  * all 7 EMA inputs the controllers consume
  * replay buffer length
  * per-step reward stats (sum, max, min, abs_max)
  * per-step done count
  * per-step action histogram (9 action classes)

Critical for cluster smoke debugging — the prior CLI only flushed
an `eprintln` progress line every N steps (default 100), making
in-flight controller drift / replay stagnation / reward explosion
invisible until they produced a NaN abort. The JSONL is line-
buffered + flushed every `log_every` steps so `tail -f` shows
progress live.

The stderr progress line is also beefed up to include γ / ε / per_α /
reward_scale / dones / rew_sum at each tick so a casual `argo logs`
inspection sees the controller behaviour without parsing JSONL.

## Why R9 cluster submission needs this

Without the diag dump, an R9 1000-step smoke is "blind" — only the
final summary tells us what happened. With the dump, post-hoc
analysis can answer:
  * Did the controllers adapt or stay at bootstrap?
  * Did the reward scale stabilise or saturate?
  * Did the PER buffer fill?
  * Was the action histogram dominated by any one action?
  * Where did the per-head losses converge to?

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-23 14:14:55 +02:00

301 lines
14 KiB
Bash
Executable File
Raw Blame History

This file contains invisible Unicode characters
This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
#!/bin/bash
# Pre-commit hook for Foxhunt HFT system
# Lightweight checks only — compilation is validated by agents before commit
set -e
echo "🔍 Running pre-commit quality checks..."
echo ""
# Check for common issues in staged files
echo "🔎 Checking for common issues..."
STAGED_FILES=$(git diff --cached --name-only --diff-filter=ACM | grep "\.rs$" || true)
if [ -n "$STAGED_FILES" ]; then
# Check for unwrap() usage
UNWRAP_FILES=$(echo "$STAGED_FILES" | xargs grep -l "\.unwrap()" 2>/dev/null || true)
if [ -n "$UNWRAP_FILES" ]; then
echo "⚠️ Warning: Found .unwrap() in staged files:"
echo "$UNWRAP_FILES" | sed 's/^/ /'
echo " Consider using ? operator or proper error handling"
echo ""
fi
# Check for expect() with generic messages
EXPECT_FILES=$(echo "$STAGED_FILES" | xargs grep -l "\.expect(\"\")" 2>/dev/null || true)
if [ -n "$EXPECT_FILES" ]; then
echo "⚠️ Warning: Found .expect() with empty message in:"
echo "$EXPECT_FILES" | sed 's/^/ /'
echo " Provide descriptive error messages"
echo ""
fi
# Check for TODO/FIXME comments
TODO_COUNT=$(echo "$STAGED_FILES" | xargs grep -c "TODO\|FIXME" 2>/dev/null | awk -F: '{sum+=$2} END {print sum}' || echo "0")
if [ "$TODO_COUNT" -gt 0 ]; then
echo " Info: Found $TODO_COUNT TODO/FIXME comments in staged files"
echo ""
fi
# Check for stub patterns in src/ files
SRC_FILES=$(echo "$STAGED_FILES" | grep "/src/" || true)
if [ -n "$SRC_FILES" ]; then
STUB_RETURNS=$(echo "$SRC_FILES" | xargs grep -n 'return 0\.0\b\|return 100\.0\|return 1\.0\b\|return vec!\[\]' 2>/dev/null | grep -v '// ok:' | grep -v '// legitimate' || true)
if [ -n "$STUB_RETURNS" ]; then
echo "⚠️ Warning: Possible stub return values in staged files:"
echo "$STUB_RETURNS" | head -5 | sed 's/^/ /'
echo " Add '// ok: <reason>' comment to suppress if intentional"
echo ""
fi
STUB_MARKERS=$(echo "$SRC_FILES" | xargs grep -n '"placeholder"\|"stub"\|"fake"\|"hardcoded"\|"dummy"' 2>/dev/null | grep -v '// ok:' || true)
if [ -n "$STUB_MARKERS" ]; then
echo "⚠️ Warning: Stub marker strings in production code:"
echo "$STUB_MARKERS" | head -5 | sed 's/^/ /'
echo ""
fi
fi
fi
# Resolve script directory by following the symlink to the real location.
# When invoked via .git/hooks/pre-commit symlink, $0 points at the symlink
# (.git/hooks/) — `dirname $0` would silently miss the sibling guard scripts
# in scripts/. `readlink -f` resolves through the symlink chain to the real path.
REAL_SCRIPT="$(readlink -f "$0")"
SCRIPT_DIR="$(cd "$(dirname "$REAL_SCRIPT")" && pwd)"
# GPU hot-path leak detection
echo "🔎 Checking for GPU→CPU leaks in hot paths..."
if [ -x "$SCRIPT_DIR/gpu-hotpath-guard.sh" ]; then
if ! "$SCRIPT_DIR/gpu-hotpath-guard.sh" --staged; then
echo "⛔ GPU hot-path leak detected — fix or suppress with // gpu-ok: <reason>"
exit 1
fi
echo " No GPU→CPU leaks in hot paths"
echo ""
else
echo "❌ Pre-commit guard missing: $SCRIPT_DIR/gpu-hotpath-guard.sh not found or not executable"
exit 1
fi
# DtoD-via-pinned guard: zero use of upload_*_via_pinned or
# clone_to_device_*_via_pinned anywhere in production code.
#
# These helpers internally do memcpy_dtod_async + stream.synchronize() — the
# "via_pinned" name is a lie. They cause CUDA_ERROR_STREAM_CAPTURE_INVALIDATED
# inside any captured graph and force a host-side stall otherwise. There is no
# legitimate use case: every CPU↔GPU communication path must use
# MappedF32Buffer / MappedI32Buffer directly (cuMemHostAlloc DEVICEMAP — host
# writes via host_ptr, kernel reads dev_ptr, zero copy).
#
# Per feedback_no_hiding: no suppression marker. If you find yourself wanting
# to suppress, rewrite the call site instead.
#
# Reference: gpu_iqn_head.rs Pearl 5 τ buffers (commit facbf76eb) — converted
# from CudaSlice + upload helper to MappedF32Buffer per-branch arrays.
check_no_dtod_via_pinned() {
local staged
staged=$(git diff --cached --name-only --diff-filter=ACM | grep '\.rs$' || true)
if [ -z "$staged" ]; then return 0; fi
local bad=""
while IFS= read -r f; do
[ -z "$f" ] && continue
# Only skip the helpers' definitions inside mapped_pinned.rs itself
# (where they currently live; the structural fix removes them entirely).
[[ "$f" == *"cuda_pipeline/mapped_pinned.rs" ]] && continue
local hits
hits=$(grep -nE 'upload_(f32|i32)_via_pinned|clone_to_device_(f32|i32)_via_pinned' "$f" 2>/dev/null \
| grep -vE '^[0-9]+:[[:space:]]*//' \
|| true)
if [ -n "$hits" ]; then
bad+="${bad:+$'\n'}$f"$'\n'"$hits"
fi
done <<< "$staged"
if [ -n "$bad" ]; then
echo "❌ DtoD-via-pinned guard violation: upload_*_via_pinned / clone_to_device_*_via_pinned"
echo " These helpers do memcpy_dtod_async + stream.synchronize() —"
echo " graph-capture-incompatible AND host-stalling. There is no"
echo " legitimate use. Rewrite to MappedF32Buffer / MappedI32Buffer"
echo " stored directly (host_ptr writes; kernel reads dev_ptr; no copy)."
echo " No suppression marker — fix the structure."
echo "$bad" | sed 's/^/ /'
return 1
fi
}
# DQN v2 Invariant 7 enforcement: component-adding commits must update audit docs.
check_audit_doc_updates() {
local staged=$(git diff --cached --name-only)
# Detect "component-adding" commits: new .rs or .cu files, or modifications to
# files that define ISV slots (gpu_dqn_trainer.rs), kernels (cuda_pipeline/*.cu),
# or state layout (state_layout.cuh).
local component_changes=$(echo "$staged" | grep -E '(^crates/ml/src/(cuda_pipeline|trainers/dqn)/|^crates/ml/src/cuda_pipeline/state_layout\.cuh$)')
if [ -z "$component_changes" ]; then return 0; fi
local audit_docs_touched=$(echo "$staged" | grep -E '^docs/(dqn-wire-up-audit|isv-slots|dqn-gpu-hot-path-audit|dqn-named-dims|ml-supervised-to-dqn-concept-audit|sp18-wireup-audit|h_s2_consumers_audit)\.md$')
if [ -z "$audit_docs_touched" ]; then
echo "❌ Invariant 7 violation: component changes without audit-doc update"
echo " Changed components:"
echo "$component_changes" | sed 's/^/ /'
echo " You must update one of:"
echo " docs/dqn-wire-up-audit.md"
echo " docs/isv-slots.md"
echo " docs/dqn-gpu-hot-path-audit.md"
echo " docs/dqn-named-dims.md"
echo " docs/ml-supervised-to-dqn-concept-audit.md"
echo " docs/sp18-wireup-audit.md"
echo " docs/h_s2_consumers_audit.md"
return 1
fi
}
# SP18 consumer-audit drift detection.
#
# When a commit touches the SP13/SP16/SP18 chain (D-leg cost-application,
# B-leg q_next bootstrap, ISV slot constants, state-reset registry, or any
# of the kernel files in cuda_pipeline that interact with these slots),
# require the locked snapshot in docs/sp18-wireup-audit.md to match the
# current run of scripts/audit_sp18_consumers.sh. This is a generalisation
# of Invariant 7: catches the SP17-style "missed quantile_q_select consumer"
# failure mode by running the audit-as-a-test against the staged diff.
#
# The script's --check mode does the work: extract the snapshot from the
# audit doc between BEGIN/END markers, diff against the live grep output,
# fail the commit if drift exceeds a tolerance (the markers themselves
# are a freshness check — if the audit doc isn't updated after a chain
# change, the markers won't have the new entries).
#
# Bypass: any commit that BOTH modifies the chain AND updates the audit
# doc (touching the markers) is allowed through — the developer made the
# scope explicit.
check_sp18_consumer_audit() {
local staged
staged=$(git diff --cached --name-only --diff-filter=ACM)
if [ -z "$staged" ]; then return 0; fi
# Trigger only on chain-relevant files
local chain_changes
chain_changes=$(echo "$staged" | grep -E '^(crates/ml/src/cuda_pipeline/(experience_kernels\.cu|hold_(rate_observer|cost_scale_update)_kernel\.cu|state_layout\.cuh|sp1[3-8]_isv_slots\.rs|sp14_isv_slots\.rs|gpu_(dqn_trainer|aux_trunk|experience_collector)\.rs)|crates/ml/src/trainers/dqn/(state_reset_registry\.rs|trainer/training_loop\.rs)|crates/ml/build\.rs|crates/ml/tests/sp1[3-8]_(oracle|phase[0-9]+_oracle)_tests\.rs)$' || true)
if [ -z "$chain_changes" ]; then return 0; fi
if [ ! -x "$SCRIPT_DIR/audit_sp18_consumers.sh" ]; then
echo " SP18 consumer-audit script not present; skipping drift check"
return 0
fi
if [ ! -f docs/sp18-wireup-audit.md ]; then
echo " docs/sp18-wireup-audit.md not present yet; skipping drift check"
return 0
fi
if "$SCRIPT_DIR/audit_sp18_consumers.sh" --check >/dev/null 2>/tmp/sp18_audit_drift.log; then
echo " SP18 consumer-audit clean (no drift vs locked snapshot)"
else
echo "❌ SP18 consumer-audit drift: chain change without audit-doc snapshot update"
echo " Changed chain files:"
echo "$chain_changes" | sed 's/^/ /'
echo " Run: scripts/audit_sp18_consumers.sh > /tmp/sp18_audit.txt"
echo " Update docs/sp18-wireup-audit.md between the BEGIN/END markers."
echo " Drift detail:"
sed 's/^/ /' /tmp/sp18_audit_drift.log
return 1
fi
}
# DQN v2 Invariant 9 enforcement: reject TODO/FIXME/XXX/HACK/TBD markers in added code.
check_no_todo_fixme() {
local added_lines=$(git diff --cached --diff-filter=AM -U0 -- '*.rs' '*.cu' '*.cuh' \
| grep -E '^\+' | grep -vE '^\+\+\+')
local bad=$(echo "$added_lines" | grep -E '(TODO|FIXME|\bXXX\b|\bHACK\b|\bTBD\b|unimplemented!|todo!\()')
if [ -n "$bad" ]; then
echo "❌ Invariant 9 violation: deferred-work markers found in staged code"
echo "$bad" | sed 's/^/ /'
return 1
fi
}
# DQN v2 Invariant (spec §4.A.2): no ISV migration functions allowed.
# The layout fingerprint is fail-fast only; no migration path exists.
check_no_isv_migrations() {
local bad=$(git diff --cached -U0 -- crates/ml/src/cuda_pipeline/gpu_dqn_trainer.rs \
crates/ml/src/trainers/dqn/trainer/ 2>/dev/null | \
grep -E '^\+.*fn\s+(migrate|upgrade)_isv' || true)
if [ -n "$bad" ]; then
echo "❌ Spec §4.A.2 violation: ISV migration functions are forbidden"
echo " (layout fingerprint is fail-fast only; no migration path exists)"
echo "$bad" | sed 's/^/ /'
return 1
fi
}
# Diff-aware guard: any NEW `stream.memcpy_htod(` or `stream.memcpy_dtoh(`
# line added in this commit is a violation of
# `feedback_no_htod_htoh_only_mapped_pinned`. The only permitted CPU↔GPU
# path is mapped-pinned staging (cuMemHostAlloc DEVICEMAP — host writes
# via `host_ptr`, kernels read `dev_ptr`, DtoD copy between them via
# `cudarc::driver::result::memcpy_dtod_async`).
#
# The existing gpu-hotpath-guard.sh explicitly skips memcpy_htod/dtoh
# (see scripts/gpu-hotpath-guard.sh:103-105) on the assumption that
# such calls only appear in cuda_pipeline/ where mapped-pinned is the
# convention. That assumption was falsified in 2026-05-23 when the R7d
# PER push/sample + R8 CLI diag landed 8 raw `stream.memcpy_htod/dtoh`
# calls in ml-alpha/src/trainer/ and ml-alpha/examples/ — the guard
# was deliberately blind to them and they shipped.
#
# This diff-aware check fires on NEW + lines only, so pre-existing
# violations elsewhere don't block commits touching unrelated files.
# Suppress per-line with `// gpu-ok: <reason>`.
check_no_raw_htod_dtoh() {
local staged
staged=$(git diff --cached --name-only --diff-filter=ACM | grep '\.rs$' || true)
if [ -z "$staged" ]; then return 0; fi
local bad=""
while IFS= read -r f; do
[ -z "$f" ] && continue
local hits
hits=$(git diff --cached -U0 -- "$f" 2>/dev/null \
| grep -E '^\+[^+]' \
| grep -E '\.memcpy_(htod|dtoh)\(' \
| grep -v 'gpu-ok:' \
|| true)
if [ -n "$hits" ]; then
bad+="${bad:+$'\n'}$f:"$'\n'"$hits"
fi
done <<< "$staged"
if [ -n "$bad" ]; then
echo "❌ feedback_no_htod_htoh_only_mapped_pinned violation: raw"
echo " stream.memcpy_htod/dtoh on a regular &[T]/&mut [T] is"
echo " forbidden. The source/dest slice is not page-locked, so the"
echo " CUDA driver does an internal blocking HtoD/DtoH copy."
echo " Use the mapped-pinned pattern instead: MappedF32Buffer /"
echo " MappedI32Buffer (host_ptr write + dev_ptr DtoD via"
echo " cudarc::driver::result::memcpy_dtod_async). See the"
echo " read_slice_d / write_slice_*_d helpers in"
echo " crates/ml-alpha/src/trainer/integrated.rs for the canonical"
echo " reference implementation."
echo ""
echo " Lines flagged (NEW additions only; pre-existing violations"
echo " are a separate cleanup tracked outside this guard):"
echo "$bad" | sed 's/^/ /'
return 1
fi
}
check_audit_doc_updates || exit 1
check_no_todo_fixme || exit 1
check_no_isv_migrations || exit 1
check_no_dtod_via_pinned || exit 1
check_no_raw_htod_dtoh || exit 1
check_sp18_consumer_audit || exit 1
echo "✅ All pre-commit checks passed!"
echo ""
echo "Summary:"
echo " - Code quality checks: ✅"
echo " - GPU hot-path guard: ✅"
echo ""
exit 0