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>
301 lines
14 KiB
Bash
Executable File
301 lines
14 KiB
Bash
Executable File
#!/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
|