Merge pull request #102 from AdaWorldAPI/claude/code-review-SMMuY

fix: close 3 storage race conditions, add 6 Level A proofs and heartb…

Author: AdaWorldAPI

.github/workflows/proof.yml

@@ -4,12 +4,13 @@
 # Runs the mathematical proof tests that verify ladybug-rs architectural
 # claims against the published literature (Berry-Esseen, NARS, Pearl, etc.)
 #
-# Three test suites:
+# Four test suites:
 #   proof_foundation       — 13 proofs (F-1 through F-7c)
 #   proof_reasoning_ladder — 8 proofs  (RL-1 through RL-8)
 #   proof_tactics          — 12 proofs (T-01 through T-34)
+#   proof_level_a_gaps     — 6 proofs  (A.1.4 through A.6.2)
 #
-# Total: 33 proofs verifying cognitive substrate invariants.
+# Total: 39 proofs verifying cognitive substrate invariants.
 # =============================================================================
 
 name: Proof Suite
@@ -66,9 +67,19 @@ jobs:
       - name: Run tactics proofs
         run: cargo test --test proof_tactics -- --test-threads=1 -v
 
+  proof-level-a-gaps:
+    name: Level A Gap Proofs (A.1.4 through A.6.2)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - uses: Swatinem/rust-cache@v2
+      - name: Run level A gap proofs
+        run: cargo test --test proof_level_a_gaps -- --test-threads=1
+
   proof-summary:
     name: Proof Summary
-    needs: [proof-foundation, proof-reasoning-ladder, proof-tactics]
+    needs: [proof-foundation, proof-reasoning-ladder, proof-tactics, proof-level-a-gaps]
     runs-on: ubuntu-latest
     if: always()
     steps:
@@ -81,10 +92,12 @@ jobs:
           echo "  Foundation:       ${{ needs.proof-foundation.result }}"
           echo "  Reasoning Ladder: ${{ needs.proof-reasoning-ladder.result }}"
           echo "  Tactics:          ${{ needs.proof-tactics.result }}"
+          echo "  Level A Gaps:     ${{ needs.proof-level-a-gaps.result }}"
           echo ""
           if [ "${{ needs.proof-foundation.result }}" = "success" ] && \
              [ "${{ needs.proof-reasoning-ladder.result }}" = "success" ] && \
-             [ "${{ needs.proof-tactics.result }}" = "success" ]; then
+             [ "${{ needs.proof-tactics.result }}" = "success" ] && \
+             [ "${{ needs.proof-level-a-gaps.result }}" = "success" ]; then
             echo "  ALL PROOF SUITES PASSED"
           else
             echo "  SOME PROOF SUITES FAILED"

examples/heartbeat.rs

@@ -0,0 +1,139 @@
+//! Heartbeat Demo — Proof of life for the ladybug-rs cognitive substrate.
+//!
+//! Exercises the core subsystems in a single pass to verify the system is
+//! alive and functioning:
+//!
+//!   1. BindSpace — 8+8 addressing, O(1) read/write
+//!   2. CogRedis  — DN.SET/DN.GET command execution
+//!   3. NARS      — Truth value revision
+//!   4. Collapse  — Gate evaluation (FLOW/HOLD/BLOCK)
+//!   5. SIMD      — Hamming distance computation
+//!
+//! Usage: `cargo run --example heartbeat`
+
+use std::time::Instant;
+
+use ladybug::core::Fingerprint;
+use ladybug::core::simd::hamming_distance;
+use ladybug::nars::TruthValue;
+use ladybug::cognitive::{get_gate_state, GateState};
+use ladybug::storage::{BindSpace, Addr, FINGERPRINT_WORDS};
+
+fn main() {
+    println!();
+    println!("==========================================================");
+    println!("  LADYBUG-RS HEARTBEAT");
+    println!("==========================================================");
+    println!();
+
+    let t0 = Instant::now();
+    let mut checks_passed = 0u32;
+    let total_checks = 5u32;
+
+    // ── 1. BindSpace ────────────────────────────────────────────────────
+    print!("  [1/5] BindSpace 8+8 addressing ... ");
+    {
+        let mut bs = BindSpace::new();
+        let addr = Addr::new(0x80, 0x01); // Node zone
+        let fp = [42u64; FINGERPRINT_WORDS];
+        bs.write_at(addr, fp);
+        let read_back = bs.read(addr);
+        assert!(read_back.is_some(), "BindSpace read after write failed");
+        let node = read_back.unwrap();
+        assert_eq!(node.fingerprint[0], 42, "Fingerprint data mismatch");
+        checks_passed += 1;
+        println!("OK  (addr={:04x})", addr.0);
+    }
+
+    // ── 2. CogRedis ────────────────────────────────────────────────────
+    print!("  [2/5] CogRedis DN.SET/DN.GET ... ");
+    {
+        use ladybug::storage::{CogRedis, RedisResult};
+        let mut redis = CogRedis::new();
+
+        // DN.SET returns the hex address of the new node
+        let result = redis.execute_command("DN.SET Ada:A:heartbeat:test hello");
+        let addr_hex = match &result {
+            RedisResult::String(s) => {
+                assert!(!s.is_empty(), "DN.SET should return address");
+                s.clone()
+            }
+            _ => panic!("DN.SET failed: {:?}", result),
+        };
+
+        // DN.GET returns an array with node info
+        let get_result = redis.execute_command("DN.GET Ada:A:heartbeat:test");
+        match &get_result {
+            RedisResult::Array(arr) => {
+                assert!(!arr.is_empty(), "DN.GET should return node info");
+            }
+            _ => panic!("DN.GET failed: {:?}", get_result),
+        }
+        checks_passed += 1;
+        println!("OK  (addr={})", addr_hex);
+    }
+
+    // ── 3. NARS Truth Value Revision ────────────────────────────────────
+    print!("  [3/5] NARS revision ............. ");
+    {
+        let tv1 = TruthValue::new(0.9, 0.8);
+        let tv2 = TruthValue::new(0.85, 0.7);
+        let revised = tv1.revision(&tv2);
+
+        // Revised confidence must be higher than either input
+        assert!(
+            revised.confidence > tv1.confidence && revised.confidence > tv2.confidence,
+            "Revision must increase confidence: {:.3} should be > {:.3} and {:.3}",
+            revised.confidence, tv1.confidence, tv2.confidence
+        );
+        // Frequency should be between inputs (weighted average)
+        assert!(
+            revised.frequency >= 0.0 && revised.frequency <= 1.0,
+            "Frequency out of range"
+        );
+        checks_passed += 1;
+        println!("OK  (f={:.3}, c={:.3})", revised.frequency, revised.confidence);
+    }
+
+    // ── 4. Collapse Gate ────────────────────────────────────────────────
+    print!("  [4/5] Collapse gate ............. ");
+    {
+        let flow = get_gate_state(0.05);
+        let hold = get_gate_state(0.25);
+        let block = get_gate_state(0.45);
+        assert_eq!(flow, GateState::Flow, "SD=0.05 should be Flow");
+        assert_eq!(hold, GateState::Hold, "SD=0.25 should be Hold");
+        assert_eq!(block, GateState::Block, "SD=0.45 should be Block");
+        checks_passed += 1;
+        println!("OK  (Flow/Hold/Block)");
+    }
+
+    // ── 5. SIMD Hamming Distance ────────────────────────────────────────
+    print!("  [5/5] SIMD hamming distance ..... ");
+    {
+        let a = Fingerprint::from_content("heartbeat_a");
+        let b = Fingerprint::from_content("heartbeat_b");
+        let d = hamming_distance(&a, &b);
+        assert!(d > 0, "Different fingerprints should have distance > 0");
+        // Self-distance must be 0
+        assert_eq!(hamming_distance(&a, &a), 0, "Self-distance must be 0");
+        checks_passed += 1;
+        println!("OK  (d={})", d);
+    }
+
+    // ── Summary ─────────────────────────────────────────────────────────
+    let elapsed = t0.elapsed();
+    println!();
+    println!("----------------------------------------------------------");
+    println!("  Result: {}/{} checks passed in {:.1}ms",
+        checks_passed, total_checks, elapsed.as_secs_f64() * 1000.0);
+
+    if checks_passed == total_checks {
+        println!("  HEARTBEAT: ALIVE");
+    } else {
+        println!("  HEARTBEAT: DEGRADED ({} failures)",
+            total_checks - checks_passed);
+    }
+    println!("==========================================================");
+    println!();
+}

src/bin/proof_report.rs

@@ -66,6 +66,18 @@ const SUITES: &[ProofSuite] = &[
             "T-34 Cross-domain fusion",
         ],
     },
+    ProofSuite {
+        name: "Level A Gaps",
+        test_name: "proof_level_a_gaps",
+        proofs: &[
+            "A.1.4 SIMD-scalar equivalence",
+            "A.2.2 NARS deduction bounds",
+            "A.3.2 Collapse gate acyclicity",
+            "A.4.3 Seven-layer fault isolation",
+            "A.5.1 Cascade search KNN correctness",
+            "A.6.2 WAL entry round-trip coverage",
+        ],
+    },
 ];
 
 fn main() {

src/storage/hardening.rs

@@ -133,19 +133,20 @@ impl LruTracker {
     }
 
     /// Record access to an address
+    ///
+    /// Duplicate-safe: removes ALL prior occurrences of `addr` from the order
+    /// queue before pushing, even if the HashMap and VecDeque were out of sync.
     pub fn touch(&mut self, addr: u16) {
         let now = SystemTime::now()
             .duration_since(UNIX_EPOCH)
             .unwrap_or_default()
             .as_micros() as u64;
 
-        // Update access time
-        if let Some(old_time) = self.access_times.insert(addr, now) {
-            // Already tracked - update order
-            if let Some(pos) = self.order.iter().position(|&a| a == addr) {
-                self.order.remove(pos);
-            }
-        }
+        self.access_times.insert(addr, now);
+
+        // Remove ALL occurrences from the order queue (not just the first)
+        // to prevent duplicate accumulation if state ever diverges.
+        self.order.retain(|&a| a != addr);
         self.order.push_back(addr);
     }
 
@@ -414,16 +415,24 @@ impl WriteAheadLog {
     }
 
     /// Append entry to WAL
+    ///
+    /// Write-ahead contract: when `sync_writes` is true, data is flushed to
+    /// the OS (and thus to disk on fsync-capable filesystems) **before** the
+    /// in-memory bookkeeping is updated. This ensures that a crash after
+    /// `append` returns will always have the entry on disk.
     pub fn append(&mut self, entry: &WalEntry) -> std::io::Result<()> {
         let bytes = entry.to_bytes();
         if let Some(file) = &mut self.file {
             file.write_all(&bytes)?;
-            self.size += bytes.len();
-            self.entries_since_checkpoint += 1;
 
+            // Flush to disk BEFORE updating in-memory state so that a crash
+            // after this point guarantees the entry is durable.
             if self.sync_writes {
                 file.flush()?;
             }
+
+            self.size += bytes.len();
+            self.entries_since_checkpoint += 1;
         }
         Ok(())
     }

src/storage/temporal.rs

@@ -376,28 +376,58 @@ impl TemporalStore {
     }
 
     /// Commit transaction
+    ///
+    /// Serializable isolation fix: the transaction is removed from `active_txns`
+    /// only **after** conflict detection and write application succeed, and the
+    /// entries/addr_index write locks are held across the entire
+    /// check-conflicts + apply-writes window so no concurrent commit can
+    /// interleave.
     pub fn commit(&self, txn_id: TxnId) -> Result<Version, TemporalError> {
-        let txn = self.active_txns.write()
-            .map_err(|_| TemporalError::LockError)?
-            .remove(&txn_id)
-            .ok_or(TemporalError::TxnNotFound(txn_id))?;
+        // Take the transaction out of active_txns only temporarily for
+        // validation; if conflict detection fails we put it back (abort path
+        // handles cleanup). We clone first so we can re-insert on failure.
+        let txn = {
+            let txns = self.active_txns.read()
+                .map_err(|_| TemporalError::LockError)?;
+            txns.get(&txn_id)
+                .cloned()
+                .ok_or(TemporalError::TxnNotFound(txn_id))?
+        };
 
         if txn.state != TxnState::Active {
             return Err(TemporalError::TxnNotActive(txn_id));
         }
 
-        // Conflict detection for Serializable
+        // Acquire write locks on entries and addr_index FIRST — hold them
+        // across conflict detection AND write application to close the
+        // TOCTOU gap that previously existed.
+        let mut entries = self.entries.write().map_err(|_| TemporalError::LockError)?;
+        let mut addr_idx = self.addr_index.write().map_err(|_| TemporalError::LockError)?;
+
+        // Conflict detection for Serializable (while holding write locks,
+        // so no other commit can modify the data we're checking against).
         if txn.isolation == IsolationLevel::Serializable {
-            self.check_conflicts(&txn)?;
+            for &addr in &txn.read_set {
+                if let Some(indices) = addr_idx.get(&addr) {
+                    for &idx in indices {
+                        if let Some(entry) = entries.get(idx) {
+                            if entry.created_version > txn.start_version {
+                                return Err(TemporalError::Conflict {
+                                    txn_id: txn.id,
+                                    addr,
+                                    conflicting_version: entry.created_version,
+                                });
+                            }
+                        }
+                    }
+                }
+            }
         }
 
-        // Advance version
+        // Advance version (after conflict check succeeds)
         let commit_version = self.versions.advance();
 
         // Apply writes
-        let mut entries = self.entries.write().map_err(|_| TemporalError::LockError)?;
-        let mut addr_idx = self.addr_index.write().map_err(|_| TemporalError::LockError)?;
-
         for (addr, mut entry) in txn.pending_writes {
             entry.created_version = commit_version;
             let idx = entries.len();
@@ -418,13 +448,24 @@ impl TemporalStore {
             }
         }
 
-        // Apply edges
+        // Apply edges (separate lock, but still under our entries lock)
         let mut edges = self.edges.write().map_err(|_| TemporalError::LockError)?;
         for mut edge in txn.pending_edges {
             edge.created_version = commit_version;
             edges.push(edge);
         }
 
+        // Drop data locks before removing from active_txns to avoid
+        // potential lock-ordering issues.
+        drop(edges);
+        drop(addr_idx);
+        drop(entries);
+
+        // NOW remove the transaction from active set (commit is durable).
+        self.active_txns.write()
+            .map_err(|_| TemporalError::LockError)?
+            .remove(&txn_id);
+
         Ok(commit_version)
     }
 
@@ -437,29 +478,9 @@ impl TemporalStore {
         Ok(())
     }
 
-    /// Check for conflicts (Serializable isolation)
-    fn check_conflicts(&self, txn: &Transaction) -> Result<(), TemporalError> {
-        let entries = self.entries.read().map_err(|_| TemporalError::LockError)?;
-
-        for &addr in &txn.read_set {
-            // Check if any entry for this addr was written after our snapshot
-            if let Some(indices) = self.addr_index.read().ok().and_then(|i| i.get(&addr).cloned()) {
-                for idx in indices {
-                    if let Some(entry) = entries.get(idx) {
-                        if entry.created_version > txn.start_version {
-                            return Err(TemporalError::Conflict {
-                                txn_id: txn.id,
-                                addr,
-                                conflicting_version: entry.created_version,
-                            });
-                        }
-                    }
-                }
-            }
-        }
-
-        Ok(())
-    }
+    // NOTE: check_conflicts logic was inlined into commit() so that it runs
+    // under the same write locks as the apply-writes phase, closing the
+    // TOCTOU gap that existed when they held separate read/write locks.
 
     // -------------------------------------------------------------------------
     // READS (with version)