Two related changes installing the structural guard against the SP6 Pearl 5 IQN τ failure mode (root cause fixed atfacbf76ebfor that one site) and removing the only remaining orphan callers of the broken pattern. The bug class. The mapped_pinned::{upload,clone_to_device}_{f32,i32}_via_pinned helpers are named to suggest "no HtoD per feedback_no_htod_htoh_only_mapped_pinned" but their bodies do MappedXBuffer::new() + memcpy_dtod_async() + stream.synchronize(). The DtoD copy and synchronize are both forbidden inside CUDA Graph capture (CUDA_ERROR_STREAM_CAPTURE_INVALIDATED) and add a host stall otherwise. The canonical pattern is MappedXBuffer stored directly + write_from_slice + kernel reads via .dev_ptr, used by SP4 portfolio_state and SP6 IQN τ atfacbf76eb. Guard. New check_no_dtod_via_pinned in pre-commit-hook.sh rejects any staged .rs file calling upload_(f32|i32)_via_pinned or clone_to_device_(f32|i32)_via_pinned, except mapped_pinned.rs itself. Per feedback_no_hiding: no suppression marker. Also fixes a pre-existing silent-skip bug: the gpu-hotpath-guard.sh invocation used $(cd "$(dirname "$0")" && pwd) which resolved to .git/hooks/ (the symlink's directory) instead of scripts/, so the guard never ran. Replaced with readlink -f "$0" + an explicit "guard missing" error branch — silent skip is worse than no guard. Orphan deletion. gpu_her.rs carried legacy relabel_batch, generate_random_donors (CPU), HerBatch, slice_clone_f32, slice_clone_i32 — zero production callers (verified via grep). Production uses relabel_batch_with_strategy + generate_random_donors_gpu. The orphan held the only upload_i32_via_pinned callers in the codebase; per feedback_no_hiding the right fix is delete. Scope. Eliminates 2 of 47 production *_via_pinned call sites. Remaining 45 across 14 files are cold-path init — graph-capture-fragile and host-stalling but not breaking operationally. Guard enforces no new calls; existing 45 migrate in subsequent atomic per-buffer commits. After all 45 are converted, the four helpers themselves get deleted from mapped_pinned.rs. Validation. Smoke smoke-test-82fjk atfacbf76ebsucceeded — magnitude differentiation restored (q_full=0.462 > q_half=0.409 > q_quarter=0.350 vs baseline frozen Pascal-triangle 0.225/0.280/0.495), eval distribution unfrozen (eq=0.596, eh=0.404, ef=0.000 vs baseline single-action collapse). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
192 lines
8.1 KiB
Bash
Executable File
192 lines
8.1 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)\.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"
|
||
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
|
||
}
|
||
|
||
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
|
||
|
||
echo "✅ All pre-commit checks passed!"
|
||
echo ""
|
||
echo "Summary:"
|
||
echo " - Code quality checks: ✅"
|
||
echo " - GPU hot-path guard: ✅"
|
||
echo ""
|
||
|
||
exit 0
|