15 KiB
SECTION 2 AUDIT REPORT
Physics Subsystem Safety & Clarity Analysis
Generated by: Claude (Sonnet 4.5) Reviewed with: Quark (GPT-5) For: Captain Bob Date: 2025-11-20
EXECUTIVE SUMMARY
I have audited the five physics subsystem files per Section 2 requirements:
- ✅
physics_metadata.c/physics_metadata.h - ✅
physics_pipelining_metrics.c - ✅
physics_hotwords_cache.c - ✅
rolling_window_of_truth.c - ⚠️
physics_runtime.c(deferred - no word_id/prev_word logic found)
Overall Status
Physics subsystem is mostly safe but needs:
- Clarifying comments explaining intent (not implementation)
- Additional bounds validation in 2 locations
- Compile-time guards around all physics features
- DOE mode CSV output verification
FINDINGS BY FILE
1. physics_metadata.c + physics_metadata.h
Safety Status: ✅ GOOD - All functions have NULL checks
NULL Check Coverage
| Line | Function | Status |
|---|---|---|
| 35-37 | physics_execution_heat_increment() |
✅ NULL check on entry |
| 45-47 | physics_execution_heat_load() |
✅ NULL check on entry |
| 55-57 | physics_decay_slope_load() |
✅ NULL check on vm |
| 59 | physics_metadata_init() |
✅ NULL check on entry |
| 71 | physics_metadata_set_mass() |
✅ NULL check on entry |
| 76 | physics_metadata_touch() |
✅ NULL check on entry |
| 167 | physics_metadata_apply_linear_decay() |
✅ NULL check on entry AND vm |
Comments Needed (Intent Clarification)
| Location | Current | Needs Comment Explaining |
|---|---|---|
| Line 35-43 | physics_execution_heat_increment() |
FL1: Heat accumulation feedback loop - why atomic operations? |
| Line 166-226 | physics_metadata_apply_linear_decay() |
FL1: Heat decay - why linear model? What's the physical analogy? |
| Line 191-193 | Decay slope Q48.16 math | Why microseconds? Why this specific slope format? |
Bounds Validation
- ✅ Line 196-198: Clamp to
cell_trange (prevents overflow) - ✅ Line 212: Check
old_heat == 0before decay (prevents underflow)
Compile-Time Guards Needed
⚠️ NONE PRESENT - All physics features compile unconditionally
Recommendation:
#if ENABLE_PHYSICS_METADATA
// ... all functions ...
#else
// Stub implementations that do nothing
#endif
2. physics_pipelining_metrics.c
Safety Status: ⚠️ MOSTLY SAFE with 2 minor issues
NULL Check Coverage
| Line | Function | Status |
|---|---|---|
| 61 | transition_metrics_init() |
✅ NULL check |
| 93 | transition_metrics_record() |
✅ NULL check on metrics |
| 113 | transition_metrics_get_probability_q48() |
✅ NULL check |
| 126 | transition_metrics_update_cache() |
✅ NULL check |
| 165 | transition_metrics_should_speculate() |
✅ NULL check |
| 287 | transition_metrics_update_context_window() |
✅ NULL check |
| 307 | transition_metrics_record_context() |
✅ NULL check |
Bounds Validation
✅ GOOD:
Line 93: if (!metrics || next_word_id >= dict_size) // Prevents out-of-bounds write
Line 307: if (...|| next_word_id >= dict_size) // Prevents out-of-bounds context record
⚠️ ISSUE #1: Unchecked array access in update_cache()
Line 136-137:
for (uint32_t i = 0; i < dict_size; i++) {
if (metrics->transition_heat[i] > 0) { // ⚠️ No guarantee transition_heat allocated to dict_size
Problem: transition_heat is lazily allocated in transition_metrics_record() with the dict_size that was passed at that time. If transition_metrics_update_cache() is called with a different (larger) dict_size, we have an out-of-bounds read.
Fix: Store allocated size in WordTransitionMetrics struct and validate.
⚠️ ISSUE #2: No bounds check in get_probability_q48()
Line 117:
uint64_t count = metrics->transition_heat ? metrics->transition_heat[target_word_id] : 0;
// ⚠️ No validation that target_word_id < allocated size
Problem: Relies on caller to pass valid target_word_id, but there's no enforcement.
Fix: Add bounds check or document precondition clearly.
Comments Needed (Intent Clarification)
| Location | Current | Needs Comment Explaining |
|---|---|---|
| Line 92-110 | transition_metrics_record() |
FL2: Word transition tracking - why record next_word_id? Purpose? |
| Line 125-148 | transition_metrics_update_cache() |
Purpose: Why cache most_likely_next_word_id? What uses this? |
| Line 164-186 | transition_metrics_should_speculate() |
FL2/FL4: Speculation decision logic - what's the ROI model? |
| Line 286-300 | transition_metrics_update_context_window() |
Purpose: Sliding window for what? How does pipelining use this? |
Compile-Time Guards Needed
⚠️ Code already uses ENABLE_PIPELINING in some places, but not consistently wrapped.
Recommendation:
#if ENABLE_PIPELINING
// ... all transition_metrics functions ...
#else
// Stub implementations
#endif
3. physics_hotwords_cache.c
Safety Status: ✅ GOOD - Comprehensive NULL and bounds checks
NULL Check Coverage
| Line | Function | Status |
|---|---|---|
| 33 | hotwords_cache_init() |
✅ NULL check on cache |
| 77 | hotwords_cache_lookup() |
✅ NULL check on cache |
| 84 | Disabled cache early exit | ✅ Safe fallback to bucket search |
| 98 | Cache iteration | ✅ NULL check on entry (line 99) |
| 173 | hotwords_cache_promote() |
✅ NULL check on cache AND word |
Bounds Validation
Line 174: if (!cache || !word || cache->cache_count >= HOTWORDS_CACHE_SIZE)
// ✅ Prevents overflow of cache array
Line 101: if ((size_t)e->name_len != len)
// ✅ Prevents length mismatch before memcmp
Line 102: if (len > 1 && ...)
// ✅ Prevents underflow on len-1
Comments Needed (Intent Clarification)
| Location | Current | Needs Comment Explaining |
|---|---|---|
| Line 76-166 | hotwords_cache_lookup() |
FL2: Cache hit/miss tracking - why track latency? Purpose of variance? |
| Line 155 | if (e->execution_heat > HOTWORDS_EXECUTION_HEAT_THRESHOLD) |
FL2: Heat-based promotion - why threshold=50? What's the feedback loop? |
| Line 173-194 | hotwords_cache_promote() |
FL2: LRU eviction - why round-robin? Why not LFU? |
Compile-Time Guards
✅ ALREADY PRESENT at line 36:
cache->enabled = ENABLE_HOTWORDS_CACHE;
But function bodies are not wrapped. Should add:
#if ENABLE_HOTWORDS_CACHE
// ... implementations ...
#else
// Return NULL / do nothing stubs
#endif
4. rolling_window_of_truth.c
Safety Status: ✅ EXCELLENT - Best defensive coding of all files
NULL Check Coverage
| Line | Function | Status |
|---|---|---|
| 57 | rolling_window_measure_diversity_view() |
✅ NULL on view AND view->history |
| 85 | rolling_window_snapshot_view() |
✅ NULL on window AND view |
| 102 | rolling_window_publish_snapshot() |
✅ NULL on window AND buffers |
| 142 | rolling_window_init() |
✅ NULL check |
| 186 | rolling_window_record_execution() |
✅ NULL check AND ->execution_history check |
| 226 | rolling_window_get_recent_sequence() |
✅ NULL on window, out_sequence, and depth==0 |
| 258 | rolling_window_find_hottest_word() |
✅ NULL AND !is_warm check |
| 303 | rolling_window_count_transition() |
✅ NULL AND !is_warm check |
Bounds Validation
Line 68: idx = (view->window_pos + ROLLING_WINDOW_SIZE - scan_limit + i) % ROLLING_WINDOW_SIZE;
// ✅ Modulo prevents out-of-bounds
Line 195: window->window_pos = (window->window_pos + 1) % ROLLING_WINDOW_SIZE;
// ✅ Circular wrap-around
Line 238: uint32_t available = (view->total_executions < ROLLING_WINDOW_SIZE) ? ...
// ✅ Clamps depth to actual data available
Line 276: if (word_id < dict_size)
// ✅ Bounds check before frequency count
Thread Safety
Line 92: __atomic_load_n(&window->snapshot_index, __ATOMIC_ACQUIRE)
Line 116: __atomic_store_n(&window->snapshot_index, write_idx, __ATOMIC_RELEASE)
// ✅ Lock-free double-buffering for reader threads
Comments Needed (Intent Clarification)
| Location | Current | Needs Comment Explaining |
|---|---|---|
| Line 184-220 | rolling_window_record_execution() |
FL1/FL2: Why record word_id? How does seeding use this? |
| Line 202-205 | Warm threshold = 1024 | Rationale: Why 1024? Statistical significance? |
| Line 412-483 | rolling_window_seed_hotwords_cache() |
FL2: Seeding strategy - why median threshold? Determinism guarantee? |
| Line 495-572 | rolling_window_seed_pipelining_context() |
FL2: Context replay - why this specific seeding algorithm? |
| Line 722-834 | rolling_window_run_adaptive_pass() |
FL3: Adaptive window sizing - why growth_rate_q48? How does shrinking work? |
Compile-Time Guards Needed
⚠️ NONE PRESENT - All rolling window code compiles unconditionally
Recommendation:
#if ENABLE_ROLLING_WINDOW
// ... all functions ...
#else
// Stub implementations
#endif
5. physics_runtime.c
Status: ⚠️ NOT AUDITED Reason: This file contains host snapshot/analytics heap infrastructure, not word_id/prev_word/transition logic.
Scope: PSI metrics, cgroup stats, analytics ring buffer - not related to Section 2 tasks.
Recommendation: Defer to Section 3 or later (if pub/sub events live here).
SAFETY ISSUES SUMMARY
Critical (Must Fix)
NONE - No critical safety vulnerabilities found.
High Priority (Should Fix)
-
⚠️
physics_pipelining_metrics.c:136-137- Unchecked array bounds intransition_metrics_update_cache()- Risk: Out-of-bounds read if
dict_sizechanges betweenrecord()andupdate_cache() - Fix: Store allocated size in struct, validate before loop
- Risk: Out-of-bounds read if
-
⚠️
physics_pipelining_metrics.c:117- No bounds check ontarget_word_idinget_probability_q48()- Risk: Out-of-bounds read if caller passes invalid
target_word_id - Fix: Add
if (target_word_id >= allocated_size) return 0;
- Risk: Out-of-bounds read if caller passes invalid
Medium Priority (Nice to Have)
-
⚠️ All files - Missing compile-time guards (
#if ENABLE_*)- Risk: Physics features cannot be disabled at compile time
- Fix: Wrap all physics code in feature flags with stub fallbacks
-
⚠️ All files - Missing intent comments (FL1, FL2, FL3, FL4 annotations)
- Risk: Future maintainers won't understand feedback loop architecture
- Fix: Add clarifying comments explaining why, not what
DOE MODE CSV OUTPUT
File: src/main.c (which is actually src/vm.c)
Function: run_doe_experiment() at line 350-392
Current Behavior
Line 381: log_message(LOG_INFO, "DoE FINAL STATE: ...") // ⚠️ Writes to stderr in DOE mode
Line 391: metrics_write_csv_row(stdout, &metrics); // ✅ Writes CSV to stdout
Issue
Line 381 log message pollutes stderr during DOE runs.
Fix
Wrap in if (!config.doe_experiment) or use LOG_DEBUG level.
Verification Needed
- Does
metrics_write_csv_row()write exactly one line with no header? - Are there any other
printf()orlog_message()calls in DOE path that leak to stdout/stderr?
RECOMMENDED CHANGES
Task 1: Add Clarifying Comments ✅
Files to modify:
physics_metadata.c- 3 comment blocksphysics_pipelining_metrics.c- 4 comment blocksphysics_hotwords_cache.c- 3 comment blocksrolling_window_of_truth.c- 5 comment blocks
Comment format:
/* ========================================================================
* INTENT: <What is the purpose of this code?>
* FL#: <Which feedback loop does this belong to?>
* WHY: <Why this approach and not alternatives?>
* ======================================================================== */
Task 2: Audit Safety (prev_word, word_id, transition_metrics) ✅
Findings:
- ✅
prev_word: Not found in physics subsystem (lives in execution loop, which is off-limits) - ✅
word_id: Bounds-checked in all critical paths - ⚠️
transition_metrics: 2 missing bounds checks (see High Priority issues above)
Files to modify:
physics_pipelining_metrics.c- Add 2 bounds checks
Task 3: Verify rolling_window Safety ✅
Status: ✅ EXCELLENT - rolling_window_of_truth.c has best safety practices of all files
No changes needed - already has:
- Comprehensive NULL checks
- Bounds validation via modulo arithmetic
- Thread-safe double-buffering
- Defensive programming throughout
Task 4: Add Compile-Time Guards ✅
Files to modify:
physics_metadata.c- Wrap in#if ENABLE_PHYSICS_METADATAphysics_pipelining_metrics.c- Wrap in#if ENABLE_PIPELININGphysics_hotwords_cache.c- Wrap in#if ENABLE_HOTWORDS_CACHErolling_window_of_truth.c- Wrap in#if ENABLE_ROLLING_WINDOW
Pattern:
#if ENABLE_FEATURE
// Real implementation
#else
// Stub that does nothing / returns safe defaults
static inline void feature_func(...) { /* no-op */ }
#endif
Task 5: DOE CSV Output ✅
File to modify:
src/main.c(a.k.a.src/vm.c)
Change:
Line 381: Change LOG_INFO to LOG_DEBUG to prevent stderr pollution in DOE mode
Verification:
- Inspect
metrics_write_csv_row()insrc/doe_metrics.c - Confirm single-line CSV output with no header
IMPACT ANALYSIS
Estimated Changes
| Category | Lines Added | Files Modified |
|---|---|---|
| Clarifying comments | ~150 | 4 |
| Compile-time guards | ~40 | 4 |
| Safety bounds checks | ~10 | 1 |
| DOE log fix | ~2 | 1 |
| TOTAL | ~202 | 5 |
Behavior Changes
ZERO - All changes are:
- Documentation (comments)
- Safety hardening (bounds checks)
- Conditional compilation (feature flags)
- Log level adjustment (DOE mode only)
No runtime behavior changes when all features are enabled (default).
NEXT STEPS
Awaiting Captain Bob's approval to proceed with:
- ✅ Add 15 clarifying comment blocks (intent, not implementation)
- ✅ Add 2 bounds checks to
physics_pipelining_metrics.c - ✅ Wrap 4 files in compile-time guards
- ✅ Fix DOE log pollution in
main.c - ✅ Verify
doe_metrics.cCSV output format
End of Section 2 Audit Report
Captain Bob & Quark - ready for your review and approval to proceed.