LithosAnanake/docs/working/archive/quality/audits/section-2-audit.md

<!-- Moved from docs/04-quality/audits/section-2-audit.md to docs/working/archive/quality/audits/section-2-audit.md on 2026-06-16 (docs reorg Phase 2) -->
# 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: <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_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.*