# 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: 1. Clarifying comments explaining **intent** (not implementation) 2. Additional bounds validation in 2 locations 3. Compile-time guards around all physics features 4. 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_t` range (prevents overflow) - ✅ Line 212: Check `old_heat == 0` before decay (prevents underflow) #### Compile-Time Guards Needed ⚠️ **NONE PRESENT** - All physics features compile unconditionally **Recommendation:** ```c #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:** ```c 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()** ```c 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()** ```c 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:** ```c #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 ```c 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: ```c cache->enabled = ENABLE_HOTWORDS_CACHE; ``` But function bodies are not wrapped. Should add: ```c #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 ```c 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 ```c 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:** ```c #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) 1. ⚠️ **`physics_pipelining_metrics.c:136-137`** - Unchecked array bounds in `transition_metrics_update_cache()` - **Risk:** Out-of-bounds read if `dict_size` changes between `record()` and `update_cache()` - **Fix:** Store allocated size in struct, validate before loop 2. ⚠️ **`physics_pipelining_metrics.c:117`** - No bounds check on `target_word_id` in `get_probability_q48()` - **Risk:** Out-of-bounds read if caller passes invalid `target_word_id` - **Fix:** Add `if (target_word_id >= allocated_size) return 0;` ### Medium Priority (Nice to Have) 3. ⚠️ **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 4. ⚠️ **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 ```c 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()` or `log_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 blocks - `physics_pipelining_metrics.c` - 4 comment blocks - `physics_hotwords_cache.c` - 3 comment blocks - `rolling_window_of_truth.c` - 5 comment blocks **Comment format:** ```c /* ======================================================================== * INTENT: * FL#: * WHY: * ======================================================================== */ ``` ### 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_METADATA` - `physics_pipelining_metrics.c` - Wrap in `#if ENABLE_PIPELINING` - `physics_hotwords_cache.c` - Wrap in `#if ENABLE_HOTWORDS_CACHE` - `rolling_window_of_truth.c` - Wrap in `#if ENABLE_ROLLING_WINDOW` **Pattern:** ```c #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:** ```c Line 381: Change LOG_INFO to LOG_DEBUG to prevent stderr pollution in DOE mode ``` **Verification:** - Inspect `metrics_write_csv_row()` in `src/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: 1. ✅ Add 15 clarifying comment blocks (intent, not implementation) 2. ✅ Add 2 bounds checks to `physics_pipelining_metrics.c` 3. ✅ Wrap 4 files in compile-time guards 4. ✅ Fix DOE log pollution in `main.c` 5. ✅ Verify `doe_metrics.c` CSV output format --- **End of Section 2 Audit Report** *Captain Bob & Quark - ready for your review and approval to proceed.*