docs(comments): correct three comments that state the opposite of the code (#101)

src/data_page.rs

@@ -159,10 +159,17 @@ impl DataPage {
     // The returned slot index is always the pre-insertion slot_count, making
     // indices monotonically increasing and stable for the page's lifetime.
     //
-    // Note (v1 simplification per ARCHITECTURE.md): the transaction layer calls
-    // PageCache::new_page() for every insert rather than scanning existing
-    // pages for free slots. Intentional, not a bug — this function itself is
-    // correct; it's just underutilized.
+    // The multi-slot machinery here is load-bearing, not spare capacity. Under
+    // R1 packing the transaction layer keeps an insert cursor — a data page
+    // allocated earlier in the SAME transaction that still has room — and every
+    // value is appended to it through this function. `PageCache::new_page` is
+    // called only when the cursor fills (this returns None) or when packing is
+    // disabled because a savepoint is active. See `transaction::packing`.
+    //
+    // This comment previously said the opposite: that the transaction layer
+    // allocated a fresh page per insert and that this function was "correct,
+    // just underutilized". Do not simplify the slot-directory append path on
+    // that basis — it now runs for every non-first insert in a transaction.
     pub fn insert(buf: &mut [u8; PAGE_SIZE], value: &[u8]) -> Option<u16> {
         let (free_start, free_end, slot_count) = Self::validate_header(buf)?;
         let needed = SLOT_ENTRY_SIZE + value.len();

src/page_cache.rs

@@ -21,11 +21,19 @@
 // - `new_page()` allocates a FRESH page_id past the current EOF. It never
 //   overwrites a live page. This is what makes copy-on-write safe: the old
 //   committed page remains untouched on disk until the superblock swap.
-// - `next_page_id` is a monotonic allocator. It is seeded from the file's
-//   page count at open time, and is bumped on every `new_page()`. Rollback
-//   does NOT rewind it (see the note in `discard`); orphaned page IDs are
-//   acceptable because they are reclaimed by the freemap after commit or
-//   simply re-truncated.
+// - `next_page_id` is a monotonic allocator WITHIN a transaction: seeded from
+//   the file's page count at open, bumped on every `new_page()`, and never
+//   rewound while a transaction is in flight. It IS rewound across a
+//   transaction boundary — `truncate()` sets it back to the truncation point
+//   (see its doc), and both `rollback` and `rollback_to` call `truncate` with
+//   the last-committed page count.
+//
+//   So a page id handed out before a rollback is NOT burned: allocate page
+//   100, roll back to a committed total of 90, allocate again, and `new_page`
+//   returns 100 a second time. Any new allocation path must therefore treat a
+//   post-rollback id as potentially aliasing a pre-rollback one and invalidate
+//   cache / spillway / freemap state for the reissued range, exactly as
+//   `truncate` already does.
 // - The cache is a STRICT bound with sidecar overflow. `load_page` evicts
 //   before insertion; `new_page` evicts after insertion. When every page
 //   in the cache is dirty, `maybe_evict` spills the LRU-tail dirty page
@@ -595,10 +603,13 @@ impl PageCache {
     /// committed pages — there is nothing to "undo" on disk, only cached
     /// garbage to throw away.
     ///
-    /// Note: `next_page_id` is deliberately NOT rewound. If rollback freed
-    /// IDs back to the allocator, two concurrent savepoint rollbacks could
-    /// hand the same ID to two different allocations. Leaving `next_page_id`
-    /// monotonic sacrifices a tiny amount of address space for correctness.
+    /// Note: this per-page path does not touch `next_page_id`, but that is a
+    /// property of `discard` alone — it is not the rollback contract. The
+    /// production rollback goes through `truncate`, which DOES rewind the
+    /// allocator; see the module header. (The rationale previously given here
+    /// — that rewinding would let "two concurrent savepoint rollbacks" hand
+    /// the same id to two allocations — never applied: the engine is
+    /// deliberately single-threaded and single-client.)
     ///
     /// `#[allow(dead_code)]`: the original rollback path called this
     /// per-page. Post-I3 (watermark rollback) the production path uses
@@ -1680,6 +1691,33 @@ mod tests {
         assert_eq!(cache.spillway.as_ref().unwrap().slot_count(), 0);
     }
 
+    #[test]
+    /// PAGE-IO-1: the module header used to assert that rollback never
+    /// rewinds `next_page_id`, so a page id handed out before a rollback could
+    /// never be reissued. `truncate` — which is what `rollback` and
+    /// `rollback_to` call — does rewind it, and the very next `new_page`
+    /// hands the same id out again. Pinned here because the false version
+    /// invited callers to skip invalidation for a reissued range.
+    fn truncate_rewinds_the_allocator_so_ids_are_reissued() {
+        let (_dir, mut cache) = fresh_cache(16);
+        for _ in 0..6 {
+            cache.new_page().unwrap();
+        }
+        assert_eq!(cache.next_page_id(), 6);
+
+        // What rollback does: truncate back to the last committed page count.
+        cache.truncate(3).unwrap();
+        assert_eq!(
+            cache.next_page_id(),
+            3,
+            "truncate must rewind the allocator, not leave it monotonic"
+        );
+
+        // And the ids really do come back around.
+        assert_eq!(cache.new_page().unwrap(), 3);
+        assert_eq!(cache.new_page().unwrap(), 4);
+    }
+
     #[test]
     fn truncate_drops_spillway_entries_above_watermark() {
         let max_pages = 2;

src/superblock/mod.rs

@@ -329,10 +329,19 @@ impl Superblock {
         buf
     }
 
-    /// Build the AAD that binds the sealed body and each key-slot's DEK wrap to
-    /// this superblock's plaintext identity. The four bootstrap fields that stay
-    /// cleartext in both encrypted and plaintext DBs are included; this prevents
-    /// transplanting a sealed body from a different DB or a different txn_counter.
+    /// Build the AAD that binds THE SEALED BODY to this superblock's plaintext
+    /// identity. The four bootstrap fields that stay cleartext in both
+    /// encrypted and plaintext DBs are included; this prevents transplanting a
+    /// sealed body from a different DB or a different txn_counter.
+    ///
+    /// Scope note: this covers the body and nothing else. The key-slot DEK
+    /// wraps do NOT use it — `wrap_dek`/`unwrap_dek` authenticate against
+    /// `KeySlot::aad()`, which is slot-local (state, kdf_id, Argon2 params,
+    /// salt, wrap_nonce) and carries no superblock identity or generation.
+    /// A key slot is therefore not cryptographically pinned to the superblock
+    /// generation it was written in. Do not read this function's existence as
+    /// evidence that it is; binding wraps to the superblock generation would
+    /// mean extending the wrap AAD, not reusing this one.
     ///
     /// These four MUST stay cleartext even in an encrypted DB precisely because
     /// they are the AAD: slot selection (`max_by_key` on `txn_counter`) and this