Files
foxhunt/scripts/pre-commit-hook.sh
jgrusewski b43413e4d3 feat(sp18 v2 P1.T1+T2): pre-dispatch consumer-audit script + pre-commit hook
Phase 1 Tasks 1.1 + 1.2 of docs/superpowers/plans/2026-05-08-sp18-reward-shape-hold-attractor.md.

Per the plan's audit-first design: surface every live reference to the
SP13/SP16 Hold-cost-scale chain (D-leg) and the TD(λ) `q_next = rewards`
self-bootstrap (B-leg) BEFORE atomic deletion. This catches the SP17-style
"missed quantile_q_select consumer" failure mode where a sweeping refactor
left a dangling reference that compiled but broke at runtime.

What lands

- `scripts/audit_sp18_consumers.sh` — 17-section grep that walks every
  consumer pattern from the plan's locked checklist (D-leg slot 380,
  461, [462..468) HCS_*, hold_cost_scale_update kernel, hold_rate_observer
  kernel retained chain, build.rs cubin manifest, state_layout.cuh
  mirror constants, state_reset_registry entries, plus B-leg
  td_lambda_kernel launch sites, q_next origin, rewards_out consumers,
  PER priority sites, replay buffer schema, c51_loss target-Q origin,
  target_params_buf consumers, PopArt slot 63 references). Three modes:
  default (full grep output), `--fingerprint` (per-section per-file hit
  count for diff-able snapshots), `--check` (diff fingerprint vs locked
  snapshot in docs/sp18-wireup-audit.md, exit 1 on drift).

- `docs/sp18-wireup-audit.md` — Phase 1 Task 1.1 outcome with two
  sections of import:
    1. The plan's locked checklist (8 entries the plan author explicitly
       identified as deletion targets).
    2. The 10 ADDITIONAL CONSUMERS surfaced by the audit (A1-A10) that
       the plan-author missed. Per the task input's halt-on-drift
       directive, Phase 1 Tasks 1.3-1.5 (atomic deletion) are HALTED
       pending human review of the expanded scope. The audit doc is the
       spec-amendment record.
    3. A locked fingerprint snapshot the pre-commit hook uses for drift
       detection on subsequent commits.
  B-leg verification confirms B-DD4 (no PER migration) + B-DD1 (target_
  params_buf reusable) + the single q_next bootstrap site at
  gpu_experience_collector.rs:4143.

- `scripts/pre-commit-hook.sh` — Invariant 7 list extended with the new
  audit doc; new `check_sp18_consumer_audit` step runs the audit script
  in `--check` mode whenever a commit touches the chain files
  (experience_kernels.cu, hold_*_kernel.cu, state_layout.cuh, sp1[3-8]_
  isv_slots.rs, gpu_dqn_trainer.rs, gpu_aux_trunk.rs, gpu_experience_
  collector.rs, state_reset_registry.rs, training_loop.rs, build.rs,
  sp1[3-8]_oracle_tests.rs). Drift triggers a hook failure with a
  pointer to the regeneration command. This is a generalisation of
  Invariant 7 and addresses Open Q-B from the spec (audit-as-pre-commit
  hook for SP-chain consumer drift).

Findings (HALT trigger)

The audit found 10 consumers NOT in the plan's locked 8-site checklist:
A1 — gpu_aux_trunk.rs:1240-1323 HoldCostScaleUpdateOps struct + impl
     (84 lines; the plan said launcher was in gpu_dqn_trainer but the
     real struct/impl lives in gpu_aux_trunk; trainer just has a thin
     forwarding method)
A2 — sp14_oracle_tests.rs:2174-2920 11 GPU oracle tests (~750 lines)
     directly exercising the deleted kernel via include_bytes!
     (sp16_phase2_hold_cost_scale_climbs_with_overrun + 10 others)
A3 — training_loop.rs:8971-9043 7 dispatch arms in reset_named_state
     (slot 461 + HCS_* slots 462-467); contract test
     every_fold_and_soft_reset_entry_has_dispatch_arm requires
     atomic deletion alongside registry entries
A4 — gpu_dqn_trainer.rs:23200-23216 constructor block writing
     HOLD_COST_BASE to slot 380 (RETAINED per DD7c, comment requires
     update)
A5 — gpu_dqn_trainer.rs:617, 2245-2256 doc-block prose
A6 — sp13_isv_slots.rs:75-77 HOLD_COST_CONTROLLER_GAIN/FLOOR/CEIL
     constants (HOLD_COST_BASE retained for A4)
A7 — gpu_experience_collector.rs:5579-5585 stale comment doc-ref
A8 — sp5_isv_slots.rs:325-327 comment doc-ref
A9 — state_reset_registry.rs:1138-1271 7 already-RETIRED entries
     promote to FULL DELETION
A10 — state_reset_registry.rs:2256-2308 lock_sp18_v2_pp4_retired_chain
      contract test asserts retired entries STILL EXIST; must be
      deleted/rewritten when entries are removed

None of these are architectural surprises — they're straight extensions
of the atomic-deletion scope. But per the task input's halt-on-drift
directive ("If your audit surfaces ANY additional consumer, halt and
report — do NOT proceed with deletion ad-hoc"), Phase 1 Tasks 1.3-1.5
HALT pending human sign-off on the expanded scope.

Path forward (next phase)

Either expand the atomic-deletion commit scope to cover all 18 sites
(8 plan + 10 audit-surfaced) — recommended per `feedback_no_partial_
refactor`; the audit doc serves as the spec-amendment record — or
human-review the audit doc and explicitly approve/reject each A* entry.

The audit doc + script + hook are the pre-condition for either path
and ship together as Tasks 1.1 + 1.2 of Phase 1. Tasks 1.3-1.6 await
human go-ahead.

Branch is at INTERIM STATE: NOT runnable for L40S smoke between this
commit and the post-Phase-1.6 close-out. The interim is purely-additive
(audit script + hook + doc); the actual deletion that creates the
"3 reward sites missing Hold cost" interim from the plan's Task 1.5
has NOT yet been performed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 02:23:57 +02:00

244 lines
11 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
}
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_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