vj-sim 0.3.0

//! Task 8 — `StateCellId` allocation and the one write primitive
//! (`commit_write`) (ledger ST-031, ST-079, ST-082, ST-091, ST-109, ST-155).
//!
//! This is the single low-level primitive every later write path (the
//! accessor ABI, `resolve_net_step`, NBA/blocking commits) goes through —
//! never reimplemented per call site (see this file's own `Acceptance`
//! grep-count discipline in the plan).
//!
//! **Deviations from the plan's literal text (documented once here, applying
//! to everything below):**
//!
//! 1. **`StateEngine`'s new fields are `pub(crate)`, not module-private.**
//!    The plan's own Files list splits `StateEngine`'s field *declarations*
//!    (`cells`, `current_time`, `current_region`, `worker_buffers`) into
//!    `state/instance.rs`, while this file's `commit_write`/`alloc_cell`
//!    (and `instance.rs`'s own `register_worker_thread`/`WorkerSlot`) need
//!    to read/write them from the *sibling* `state::write` module. Rust's
//!    privacy model has no "private to a set of sibling modules" tier
//!    between "private to the declaring module (and its descendants)" and
//!    "visible everywhere `pub` reaches" other than `pub(crate)`, so the
//!    fields (and `WorkerSlot`'s inner index) are `pub(crate)`: still fully
//!    outside `vj-sim`'s public API surface (the encapsulation the plan
//!    actually cares about), just visible across this one crate's own
//!    sibling modules.
//! 2. **A fourth, new `engine_id: u64` field on `StateEngine`** (assigned
//!    once, monotonically, from a process-wide atomic counter, at
//!    `StateEngine::new`). The plan's own thread-local self-registration
//!    cache is keyed purely by "is this thread's cache populated at all",
//!    which is sound only if at most one `StateEngine` is ever built per OS
//!    thread for that thread's entire lifetime — true for a real
//!    simulation run's worker threads, but not for this crate's own test
//!    suite (many short-lived `StateEngine`s; Rust's default test harness
//!    does not guarantee a fresh OS thread per `#[test]` function). Without
//!    `engine_id`, a second `StateEngine` built on a thread that already
//!    populated the cache for a *different*, since-dropped engine would
//!    reuse that engine's dangling raw pointer — a genuine use-after-free,
//!    not merely a wrong-value bug. Keying the cache by `engine_id` instead
//!    closes this while keeping the plan's "no further registry lookup
//!    once cached" cost profile and leaving every call site's own
//!    signature/behavior unchanged.
//! 3. **`VcRecord` derives `Debug, Clone` only, not `PartialEq`; `ChangeResult`
//!    derives `Debug` only, no `PartialEq`.** Both embed [`FourStateValue`],
//!    which this crate's Task 1 deliberately never gives a `PartialEq` impl
//!    (see `state::value`'s/`SlotDescriptor`'s doc comments and this task's
//!    own ST-155 discussion below) precisely so no shortcut/value-only
//!    equality check can silently exist — `#[derive(PartialEq)]` cannot be
//!    added to a struct one of whose fields has no `PartialEq` impl, so the
//!    plan's literal derive lists for these two types do not compile as
//!    written. Tests that would have used `assert_eq!` on a whole
//!    `ChangeResult`/`VcRecord` instead assert on `.changed`/`.cell`/`.at`
//!    directly and pattern-match the `FourStateValue` fields.
//!
//! **Task 9 retrofit (ST-013, ST-020..ST-023 — not in the plan's Files list
//! for Task 9, which names only `accessor_impl.rs` (new) and `mod.rs`
//! (edit)):** the plan's header design decision makes `SimHandle` point at a
//! bare `InstanceHandle{base, template, arena, layout}` with no back
//! -reference to the `StateEngine` that minted it, yet also requires
//! `sv_set_scalar`/`sv_set_wide` to "call Task 8's `StateEngine::commit_write`
//! (not a second, parallel write path)". Those two requirements are only
//! reconcilable if whatever `commit_write` needs beyond `Arena` (itself
//! already `Rc`-shared and reachable with no `StateEngine` back-reference,
//! per Task 7 r2) is *also* reachable without one. [`VcState`] below pulls
//! `current_time`/`current_region`/`worker_buffers`/`engine_id` out of
//! `StateEngine` (`state::instance`, which now holds `vc: Rc<VcState>`
//! instead of those four fields) into their own `Rc`-shared, interior
//! -mutable type, mirroring the exact `Rc`+`UnsafeCell`/`&self`-accessor
//! pattern Task 7 r1/r2 already established for `Arena`/`TemplateRegistry`,
//! for the identical reason: `InstanceHandle` (Task 9 retrofit,
//! `state::instance`) clones this same `Rc<VcState>` at `allocate_instance`
//! time, so the accessor ABI (`accessor_impl.rs`) can call
//! [`commit_write_at`] — the same diff-and-append primitive
//! `StateEngine::commit_write` now merely forwards to — directly, with no
//! `&mut StateEngine` in scope. `StateEngine`'s own public methods
//! (`set_current_time`, `set_current_region`, `register_worker_thread`,
//! `commit_write`) keep their exact pre-Task-9 signatures; only their bodies
//! now forward through `self.vc`.

use std::cell::{Cell, RefCell, UnsafeCell};

use vj_abi::{Region, SimTime};

use super::arena::{Arena, BuildMode};
use super::instance::StateEngine;
use super::{AliasWindow, EdgeKind, FourStateValue, SlotClass, StateCellId};

/// The already-*absolute* (base already added) address [`StateEngine::commit_write`]
/// reads/writes at; both `InstanceHandle`-relative accessor calls and
/// `StateCellId`-keyed calls resolve down to this shape before calling in.
///
/// **Task 11 (ST-137, ST-138) adds `row_stride_words`/`num_rows`,** mirroring
/// [`super::SlotOffset`]'s own field suffix (ST-144) — a `SlotClass::Memory`
/// slot's own per-row word stride and total row count, both stable
/// template constants copied straight through from the owning `SlotOffset`
/// (never recomputed here). Every non-memory `SlotLocation` this crate builds
/// sets both to `0` (mechanical ripple, matching `SlotOffset`'s own
/// established convention for a non-memory slot).
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub struct SlotLocation {
    pub class: SlotClass,
    pub word_offset: u32,
    pub bit_width: u32,
    pub elem_word_count: u32,
    pub row_stride_words: u32,
    pub num_rows: u32,
}

/// Task 11 (ST-138): state.md §3's pinned formula, verbatim: `word_offset' =
/// word_offset + row_stride_words * row_index`, `row_index` a *runtime*
/// value. Panics (never a silent OOB read/write) if `loc.class !=
/// SlotClass::Memory` or `row_index >= loc.num_rows as u64` — ARCHITECTURE §7
/// item 9's "loud, never silent" discipline. The returned `SlotLocation`
/// still has `class: SlotClass::Memory` and the SAME `elem_word_count` (the
/// layout builder already sets `elem_word_count == row_stride_words` for
/// every real `Memory` slot — one row's own word count — so the resolved
/// location slots directly into the existing `read_slot`/`write_slot`/
/// `commit_write_at` primitives with zero further change).
///
/// **Task 11 deviation:** `pub`, not `pub(crate)` — see [`read_slot`]'s own
/// doc comment for why.
pub fn resolve_mem_row(loc: SlotLocation, row_index: u64) -> SlotLocation {
    assert!(
        loc.class == SlotClass::Memory,
        "resolve_mem_row: SlotLocation class {:?} is not SlotClass::Memory -- \
         row addressing only ever applies to a memory slot",
        loc.class
    );
    assert!(
        row_index < loc.num_rows as u64,
        "resolve_mem_row: row index {row_index} is out of bounds for a {}-row memory",
        loc.num_rows
    );
    let word_offset = loc.word_offset as u64 + loc.row_stride_words as u64 * row_index;
    SlotLocation {
        word_offset: u32::try_from(word_offset)
            .expect("resolve_mem_row: resolved word_offset overflows u32"),
        ..loc
    }
}

/// ST-109's exact shape: `commit_write`'s return value.
#[derive(Debug)]
pub struct ChangeResult {
    pub changed: bool,
    pub old: FourStateValue,
    pub new: FourStateValue,
}

/// The VC-buffer's one record shape (ST-079, ST-109) — later `drain_committed_changes`
/// consumes this type as-is, never redefined.
#[derive(Debug, Clone)]
pub struct VcRecord {
    pub cell: StateCellId,
    pub old: FourStateValue,
    pub new: FourStateValue,
    pub at: SimTime,
}

/// Opaque handle to one thread's registered speculative VC buffer, returned
/// by [`StateEngine::register_worker_thread`]. `pub(crate)` inner index: see
/// this module's top doc comment, deviation 1.
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub struct WorkerSlot(pub(crate) usize);

/// Task 9 retrofit (see this module's top doc comment): the subset of
/// `StateEngine`'s own state `commit_write_at` needs beyond `Arena`, pulled
/// into its own `Rc`-shared, interior-mutable type so both `StateEngine`
/// (`state::instance`) and every `InstanceHandle` minted against it can call
/// the same primitive without a `&mut StateEngine` back-reference.
pub(crate) struct VcState {
    current_time: Cell<SimTime>,
    current_region: Cell<Region>,
    // See `StateEngine`'s pre-Task-9 identical field for why this is boxed
    // per-entry (`clippy::vec_box` false positive, same reasoning).
    #[allow(clippy::vec_box)]
    worker_buffers: UnsafeCell<Vec<Box<RefCell<Vec<VcRecord>>>>>,
    engine_id: u64,
    /// Task 12 (ST-081): the last value `StateEngine::advance_gvt` stored —
    /// `advance_gvt` itself is a pure setter (`self.vc.set_gvt_watermark(gvt)`),
    /// with no dependency on any scheduler-internal type. `drain_committed_changes`
    /// (below) is given its own `up_to: SimTime` parameter directly by the
    /// caller rather than reading this field back internally (matching this
    /// task's pinned `drain_committed_changes(&mut self, up_to: SimTime)`
    /// signature) — this field exists so `advance_gvt` has real, observable
    /// state to set (proven by the `#[cfg(test)]` getter below), not so
    /// `drain_committed_changes` consults it.
    gvt_watermark: Cell<SimTime>,
    /// `force`/`release` (`state::force`): this engine's forced cells.
    pub(crate) force: RefCell<super::force::ForceTable>,
    /// Whether `commit_write_at` appends a `VcRecord` per change (ST-124).
    /// Only `drain_committed_changes` consumes those records, so a run with
    /// no such consumer turns this off (`StateEngine::set_change_log`):
    /// otherwise every value change of the whole run stays buffered.
    record_changes: Cell<bool>,
    /// Window aliases of ref ports bound to a select (`state::alias`).
    pub(crate) views: super::alias::Views,
}

impl VcState {
    pub(crate) fn new(engine_id: u64) -> Self {
        VcState {
            current_time: Cell::new(SimTime {
                sim_units: 0,
                delta: 0,
            }),
            current_region: Cell::new(Region::Active),
            worker_buffers: UnsafeCell::new(Vec::new()),
            engine_id,
            gvt_watermark: Cell::new(SimTime {
                sim_units: 0,
                delta: 0,
            }),
            force: RefCell::new(super::force::ForceTable::default()),
            record_changes: Cell::new(true),
            views: super::alias::Views::default(),
        }
    }

    pub(crate) fn records_changes(&self) -> bool {
        self.record_changes.get()
    }

    pub(crate) fn set_record_changes(&self, on: bool) {
        self.record_changes.set(on);
    }

    pub(crate) fn set_current_time(&self, at: SimTime) {
        self.current_time.set(at);
    }

    pub(crate) fn set_current_region(&self, region: Region) {
        self.current_region.set(region);
    }

    /// Task 12: `commit_write` reads this back to stamp `notify_scheduler`'s
    /// `at`/`region` arguments with the same `SimTime`/`Region` `VcRecord`
    /// appends use (this plan's header "how STATE learns now" decision).
    pub(crate) fn current_time(&self) -> SimTime {
        self.current_time.get()
    }

    pub(crate) fn current_region(&self) -> Region {
        self.current_region.get()
    }

    /// Task 12 (ST-081): see this type's `gvt_watermark` field doc comment.
    pub(crate) fn set_gvt_watermark(&self, gvt: SimTime) {
        self.gvt_watermark.set(gvt);
    }

    #[cfg(test)]
    pub(crate) fn gvt_watermark(&self) -> SimTime {
        self.gvt_watermark.get()
    }

    /// Task 8's "cross-thread visibility of the VC speculative buffer"
    /// header decision, now `&self`-based (see this type's doc comment):
    /// pushes a fresh, individually-boxed `RefCell<Vec<VcRecord>>` onto
    /// `worker_buffers` and returns its index -- each entry's own heap
    /// allocation has a stable address regardless of how many further times
    /// `worker_buffers` itself grows (the same box-per-entry stability
    /// argument `TemplateRegistry` already relies on for `StateLayout`).
    ///
    /// # Safety
    /// The caller must ensure no other live borrow of `worker_buffers` (in
    /// particular, no live dereference of a pointer from a previous
    /// [`Self::buffer_ptr`] call) is held across this call, and that at most
    /// one logical writer grows this list at a time -- the same
    /// single-logical-writer discipline `Arena`'s own growing accessors
    /// document (see `state::arena`'s module doc comment). `&self`, not
    /// `&mut self`, so this is callable through an `Rc<VcState>` without ever
    /// forming a `&mut VcState`.
    pub(crate) unsafe fn register_worker_thread(&self) -> WorkerSlot {
        // SAFETY: forwarded to this function's own `# Safety` section above.
        unsafe {
            let buffers = &mut *self.worker_buffers.get();
            buffers.push(Box::new(RefCell::new(Vec::new())));
            WorkerSlot(buffers.len() - 1)
        }
    }

    /// Raw pointer to the (stably-heap-addressed) buffer registered at
    /// `slot` -- see [`Self::register_worker_thread`]'s doc comment for why
    /// growing `worker_buffers` later never invalidates this pointer.
    ///
    /// # Safety
    /// `slot` must have been returned by a prior `register_worker_thread`
    /// call on this same `VcState`, and no concurrent `register_worker_thread`
    /// call may race this read.
    unsafe fn buffer_ptr(&self, slot: WorkerSlot) -> *const RefCell<Vec<VcRecord>> {
        // SAFETY: forwarded to this function's own `# Safety` section above.
        let buffers: &Vec<Box<RefCell<Vec<VcRecord>>>> = unsafe { &*self.worker_buffers.get() };
        &*buffers[slot.0]
    }

    /// Task 12 (ST-078, ST-081; review fix — this plan's header "cross-thread
    /// visibility" decision): drains **every** registered worker thread's
    /// buffer (not only the calling thread's) of records with `at <= up_to`,
    /// merging the removed records into one `Vec<VcRecord>` ordered by
    /// `(at, registration index)`. With exactly 1 registered worker this
    /// degenerates to the old single-buffer behavior; with N it is correct
    /// for all N — Waveform/VPI/`$display` observe exclusively through this
    /// method (`StateEngine::drain_committed_changes`, `state::gvt`), never
    /// through `Dispatch::notify_waveform_and_vpi` eagerly.
    ///
    /// Each registered buffer is walked in registration order and its
    /// matching records are `Vec::remove`d in place (so a record already
    /// drained never appears again on a later call, and a record with
    /// `at > up_to` is left in the buffer for a future call once the
    /// watermark passes it); the concatenated result is then stable-sorted
    /// by `at` alone — stable sort preserves the `(buffer-registration-order,
    /// within-buffer-append-order)` relative order of any two records that
    /// tie on `at`, which is exactly "registration index" as a secondary key.
    ///
    /// # Safety precondition (documented, not enforced by the type system)
    /// May run only when no worker thread holds a live borrow of its own
    /// `RefCell<Vec<VcRecord>>` (i.e. is not concurrently inside a
    /// `commit_write_at` call) — the same WOCB-barrier precondition
    /// documented on [`Self::register_worker_thread`]. `&self`, not `&mut
    /// self`/`&mut StateEngine`, only for symmetry with this type's other
    /// methods; `StateEngine::drain_committed_changes` (`state::gvt`) still
    /// takes `&mut self` itself, which is what actually enforces
    /// non-reentrancy from a single caller's own perspective.
    pub(crate) fn drain_committed_changes(&self, up_to: SimTime) -> Vec<VcRecord> {
        // SAFETY: `worker_buffers` only ever grows by `push` (never
        // reallocates/moves an existing `Box`'s heap allocation, per
        // `register_worker_thread`'s own doc comment), and this call's own
        // `# Safety precondition` above rules out a concurrent writer
        // borrowing any entry while this runs.
        let buffers: &Vec<Box<RefCell<Vec<VcRecord>>>> = unsafe { &*self.worker_buffers.get() };

        let mut merged: Vec<VcRecord> = Vec::new();
        for buf in buffers.iter() {
            let mut records = buf.borrow_mut();
            let mut i = 0;
            while i < records.len() {
                if sim_time_le(records[i].at, up_to) {
                    merged.push(records.remove(i));
                } else {
                    i += 1;
                }
            }
        }
        // Stable sort: see this method's own doc comment for why this alone
        // realizes the "(at, registration index)" ordering.
        merged.sort_by_key(|a| (a.at.sim_units, a.at.delta));
        merged
    }
}

/// Total order over `SimTime` (`sim_units` then `delta`) this crate needs for
/// GVT-watermark comparisons — `vj_abi::SimTime` itself derives no
/// `PartialOrd`/`Ord` (see `handle.rs`; out of this track's contract to add
/// one to another crate's type), so this is a local helper, not a trait impl.
///
/// `pub(crate)` (Task 16 deviation): `state::rollback::UndoLog::fossil_collect`
/// needs the exact same total order to compare an entry's own `at` against a
/// `gvt` watermark — reusing this rather than a second, independently
/// hand-rolled comparator (the same "one place" discipline `drain_committed_changes`
/// above already follows for its own `at <= up_to` gating).
pub(crate) fn sim_time_le(a: SimTime, b: SimTime) -> bool {
    (a.sim_units, a.delta) <= (b.sim_units, b.delta)
}

/// The ST-091/ST-031 "hook present in the calling convention from the first
/// freeze" requirement made concrete now: `commit_write` takes `hook: &mut
/// dyn PreWriteHook` as a required parameter, not an optional add-on, so a
/// later real undo-log `PreWriteHook` implementer slots in without touching
/// this signature or any of its call sites.
///
/// **Deviation from the plan's literal signature** (binding per this task's
/// own unresolved-review-findings list): `before_write` also receives
/// `cell: StateCellId`, not just `loc`/`old`/`new_incoming` — `commit_write`
/// already has `cell` in scope at the call site, and a later undo-log
/// implementer needs the `StateCellId` (not merely a `SlotLocation`) to
/// replay a rollback via `commit_write_no_hook(cell, ...)`; no
/// `SlotLocation -> StateCellId` reverse lookup exists anywhere in this
/// crate, so `cell` must be threaded through here directly.
///
/// **Second deviation (Task 16):** `before_write` also receives
/// `at: SimTime`. Task 16's `UndoLog` (`state::rollback`) implements this
/// trait and must tag every entry it records with the `SimTime` the write
/// happened at — `fossil_collect(gvt)`'s own acceptance text ("trims entries
/// the caller asserts are provably before `gvt`") is only meaningful if the
/// log's entries actually carry a timestamp to compare against `gvt`, and
/// nothing else in this calling convention threads one to the hook. The one
/// call site (`commit_write_at`, below) already reads `vc.current_time()`
/// for its own `VcRecord.at` field, so passing that same value to the hook
/// costs nothing extra and keeps both timestamps (the VC record's and the
/// undo entry's) identically sourced.
pub trait PreWriteHook {
    fn before_write(
        &mut self,
        cell: StateCellId,
        loc: SlotLocation,
        old: &FourStateValue,
        new_incoming: &FourStateValue,
        at: SimTime,
    );
}

/// The no-op default: v1 (`LOOKAHEAD_CYCLES = 0`) never exercises rollback,
/// so every call site that doesn't care passes `&mut ()`.
impl PreWriteHook for () {
    fn before_write(
        &mut self,
        _cell: StateCellId,
        _loc: SlotLocation,
        _old: &FourStateValue,
        _new_incoming: &FourStateValue,
        _at: SimTime,
    ) {
    }
}

/// One thread's cached self-registration: which `StateEngine` (`engine_id`,
/// deviation 2 above) this thread last registered against, the `WorkerSlot`
/// that registration returned, and a raw pointer straight at that boxed
/// entry's `RefCell` (stable heap address — see `register_worker_thread`'s
/// doc comment), so every subsequent `commit_write` on this thread against
/// the *same* engine appends with no further registry lookup.
struct WorkerCacheEntry {
    engine_id: u64,
    #[allow(dead_code)]
    // kept for debugging/symmetry with the plan's named field; `buffer` is what every call site actually uses.
    slot: WorkerSlot,
    buffer: *const RefCell<Vec<VcRecord>>,
}

thread_local! {
    static WORKER_CACHE: RefCell<Option<WorkerCacheEntry>> = const { RefCell::new(None) };
}

/// ST-155, hand-written (see this module's top doc comment, deviation 3, for
/// why `FourStateValue` cannot derive `PartialEq`): the one place that diffs
/// old-vs-new, independently comparing **both** the value/aval half **and**
/// the mask/unknown/bval half for **both** storage tiers — never a
/// value-only or single-plane shortcut. `Narrow`'s low 64 bits are the value
/// half, the high 64 bits are the mask half (state::value's fixed accessor-ABI
/// packing); `Wide`'s `aval`/`bval` planes are compared element-wise,
/// independently of each other.
pub(crate) fn four_state_changed(old: &FourStateValue, new: &FourStateValue) -> bool {
    match (old, new) {
        (FourStateValue::Narrow(o), FourStateValue::Narrow(n)) => {
            let o_val = *o as u64;
            let o_unk = (*o >> 64) as u64;
            let n_val = *n as u64;
            let n_unk = (*n >> 64) as u64;
            o_val != n_val || o_unk != n_unk
        }
        (
            FourStateValue::Wide { aval: oa, bval: ob },
            FourStateValue::Wide { aval: na, bval: nb },
        ) => oa != na || ob != nb,
        (FourStateValue::Narrow(_), FourStateValue::Wide { .. })
        | (FourStateValue::Wide { .. }, FourStateValue::Narrow(_)) => {
            // A cell's `SlotLocation::class` fixes which `FourStateValue`
            // shape it ever stores (`Inline` <-> `Narrow`, `Wide`/`Memory`
            // <-> `Wide`); a caller reaching this arm passed a value whose
            // shape doesn't match the cell's own storage class, which is a
            // programming error at the call site, not a normal divergence
            // this primitive is meant to tolerate silently.
            panic!(
                "commit_write: old/new FourStateValue shape mismatch (Narrow vs Wide) for the \
                 same cell -- the caller's `new` value does not match this cell's storage class"
            );
        }
    }
}

/// Reads the current word(s) stored at `loc`, without any diffing —
/// `commit_write_at`'s own `old` read (Task 9 retrofit: a free function over
/// `&Arena`, not a `StateEngine` method, so it is callable from the accessor
/// ABI's `InstanceHandle`-only call sites too — see this module's top doc
/// comment).
///
/// **Task 11 deviation (widened from `pub(crate)` to `pub`):** the plan's own
/// `mem_row_addressing_uses_real_frontend_layout.rs` acceptance test (ST-138)
/// is a genuine external integration test (`crates/vj-sim/tests/`, a
/// separate compilation unit that sees only this crate's `pub` surface) and
/// needs to read back a resolved memory row's words directly, through the
/// same no-second-path primitive `commit_write_at` itself uses, to prove
/// `resolve_mem_row`'s output slots into it correctly on the real,
/// frontend-derived layout — not a hand-rolled second reader. Widening this
/// (and `write_slot`/`resolve_mem_row` below) to `pub` only enlarges this
/// crate's public surface to match the already-`pub` sibling 2-state family
/// (`read_inline_2s`/`write_inline_2s`/`read_wide_2s`/`write_wide_2s`, this
/// same file, Task 13) that already sits at this exact visibility for the
/// identical reason — it does not change behavior or the single-writer
/// discipline either family already documents.
pub fn read_slot(arena: &Arena, loc: SlotLocation) -> FourStateValue {
    match loc.class {
        SlotClass::Inline => FourStateValue::Narrow(arena.inline()[loc.word_offset as usize]),
        // `Memory` shares the wide planes with `Wide` (arena.rs's own
        // module doc comment: "wide_aval/wide_bval hold every wider
        // signal and every memory element"); by the time a `SlotLocation`
        // exists, any per-row addressing a memory access needed has
        // already resolved down to a plain word_offset/elem_word_count
        // range in those planes, same as an ordinary wide signal.
        SlotClass::Wide | SlotClass::Memory => {
            let start = loc.word_offset as usize;
            let end = start + loc.elem_word_count as usize;
            FourStateValue::Wide {
                aval: arena.wide_aval()[start..end].to_vec(),
                bval: arena.wide_bval()[start..end].to_vec(),
            }
        }
        // Review fix (Task 14 r2, blocking finding 2): `force_driver`'s
        // Wide/Memory-class contribution cells live in `Arena`'s separate,
        // never-sealed `force_wide_aval`/`force_wide_bval` planes, not the
        // ordinary wide planes — see `SlotClass::ForceWide`'s own doc
        // comment.
        SlotClass::ForceWide => {
            let start = loc.word_offset as usize;
            let end = start + loc.elem_word_count as usize;
            FourStateValue::Wide {
                aval: arena.force_wide_aval()[start..end].to_vec(),
                bval: arena.force_wide_bval()[start..end].to_vec(),
            }
        }
    }
}

/// Stores `new` at `loc`. Only `commit_write_at` (below) calls this in the
/// ordinary write path, and only after `four_state_changed` has already
/// confirmed a real change — see this function's own SAFETY comments for the
/// single-logical-writer argument that makes the two `unsafe` `Arena`
/// accessor calls sound.
///
/// `pub(crate)` (Task 16 deviation, not in that task's own Files list):
/// `state::rollback`'s `StateEngine::commit_write_no_hook` (the undo-log's
/// own reverse-replay primitive) and `StateEngine::reset_to_initial`/
/// `restore` need this exact same raw, no-diff, no-hook, no-VC-record store
/// — reusing it rather than a second hand-rolled copy of the same
/// class-dispatch (ST-155's "one place" discipline applied to writes, not
/// just diffing).
///
/// **Task 11 deviation:** widened `pub(crate)` -> `pub` for the same reason,
/// and by the same precedent, as [`read_slot`]'s own doc comment above.
pub fn write_slot(arena: &Arena, loc: SlotLocation, new: &FourStateValue) {
    match (loc.class, new) {
        (SlotClass::Inline, FourStateValue::Narrow(word)) => {
            // SAFETY: every caller of `commit_write_at` upholds the
            // single-logical-writer discipline `Arena`'s own mutating
            // accessors document (`StateEngine::commit_write` under its own
            // `&mut self`; the accessor ABI's `sv_set_scalar`/`sv_set_wide`
            // under the accessor-ABI's documented single-writer-per-slot
            // calling convention) -- nothing in this crate holds a live
            // `&[u128]`/`&mut [u128]` borrow of the inline plane across this
            // call (mirrors Task 7's `allocate_instance` SAFETY argument for
            // the same `Arena`).
            unsafe {
                arena.inline_mut()[loc.word_offset as usize] = *word;
            }
        }
        (SlotClass::Wide | SlotClass::Memory, FourStateValue::Wide { aval, bval }) => {
            let start = loc.word_offset as usize;
            // A huge signal's mirror must not outlive a write it did not make
            // (`state::huge_mirror`); a plain compare when no mirror exists.
            arena.huge.invalidate_range(start, aval.len());
            // SAFETY: same single-logical-writer argument as the Inline arm
            // above, applied to both wide planes; `aval.len() == bval.len()
            // == loc.elem_word_count` by construction of every
            // `FourStateValue::Wide` this crate ever builds for a
            // `Wide`/`Memory`-class cell (`read_slot` above is the only
            // producer of a `Wide` for such a cell via `commit_write_at`,
            // and always slices exactly `elem_word_count` words from each
            // plane; the accessor ABI's `sv_set_wide` builds its own
            // `Wide` the same way, from the caller-supplied `WideView`'s
            // `word_count` words).
            unsafe {
                arena.wide_aval_mut()[start..start + aval.len()].copy_from_slice(aval);
                arena.wide_bval_mut()[start..start + bval.len()].copy_from_slice(bval);
            }
        }
        // Review fix (Task 14 r2, blocking finding 2): same shape as the
        // `Wide | Memory` arm above, through the separate never-sealed
        // force-only planes — see `SlotClass::ForceWide`'s own doc comment.
        (SlotClass::ForceWide, FourStateValue::Wide { aval, bval }) => {
            let start = loc.word_offset as usize;
            // SAFETY: same single-logical-writer argument as the `Wide |
            // Memory` arm above, applied to the force-only wide planes;
            // `aval.len() == bval.len() == loc.elem_word_count` by the same
            // construction argument (this crate's only producer of a `Wide`
            // shape for a `ForceWide`-class cell is `read_slot` above, via
            // `force_driver`'s own `commit_write` seed and later
            // `resolve_net_step` reads).
            unsafe {
                arena.force_wide_aval_mut()[start..start + aval.len()].copy_from_slice(aval);
                arena.force_wide_bval_mut()[start..start + bval.len()].copy_from_slice(bval);
            }
        }
        (class, value) => panic!(
            "commit_write: SlotClass {class:?} does not match the FourStateValue shape \
             being written ({value:?}) -- a caller passed a value shape that doesn't match \
             this cell's own storage class"
        ),
    }
}

/// Returns this thread's cached raw pointer into `vc`'s own registered
/// worker buffer, calling [`VcState::register_worker_thread`] itself, at
/// most once per (thread, engine) pair — see this module's top doc comment,
/// deviation 2, for why the cache is keyed by `engine_id` and not merely "is
/// this thread's cache populated".
fn thread_local_buffer(vc: &VcState) -> *const RefCell<Vec<VcRecord>> {
    let cached = WORKER_CACHE.with(|cache| {
        cache
            .borrow()
            .as_ref()
            .filter(|entry| entry.engine_id == vc.engine_id)
            .map(|entry| entry.buffer)
    });
    if let Some(ptr) = cached {
        return ptr;
    }

    // SAFETY: the cache check above ensures this runs at most once per
    // (thread, `vc`) pair; no other live borrow of `vc`'s `worker_buffers`
    // is held across this call.
    let slot = unsafe { vc.register_worker_thread() };
    // SAFETY: `slot` was just returned by `register_worker_thread` above, on
    // this same `vc`; per that method's own doc comment, its heap allocation
    // stays stable regardless of later registrations.
    let ptr = unsafe { vc.buffer_ptr(slot) };
    WORKER_CACHE.with(|cache| {
        *cache.borrow_mut() = Some(WorkerCacheEntry {
            engine_id: vc.engine_id,
            slot,
            buffer: ptr,
        });
    });
    ptr
}

/// The one write primitive (ST-109, ST-079, ST-082, ST-091, ST-155; Task 9
/// retrofit — see this module's top doc comment): reads the current value,
/// calls `hook.before_write` *before* mutating (ST-091), diffs old-vs-new on
/// both halves independently (ST-155); if (and only if) different, stores
/// `new` and appends one `VcRecord` (stamped with `vc`'s own `current_time`)
/// to the calling thread's own registered speculative buffer (ST-079,
/// ST-082). A free function over `&Arena`/`&VcState` (not a `StateEngine`
/// method) so both `StateEngine::commit_write` (keyed by `StateCellId`,
/// resolved via `self.cells`) and the accessor ABI (Task 9, keyed directly
/// by an already-absolute `SlotLocation` resolved via `InstanceHandle`) call
/// this exact same primitive — never a second, parallel write path.
pub(crate) fn commit_write_at(
    arena: &Arena,
    vc: &VcState,
    loc: SlotLocation,
    cell: StateCellId,
    new: FourStateValue,
    hook: &mut dyn PreWriteHook,
) -> ChangeResult {
    // A forced cell keeps its forced bits (`state::force`).
    let new = {
        let mut forced = vc.force.borrow_mut();
        if forced.is_empty() {
            new
        } else {
            if loc.class == SlotClass::Memory {
                forced.filter_row(cell, loc.word_offset, new)
            } else {
                forced.filter(cell, new)
            }
        }
    };
    let old = read_slot(arena, loc);
    // Task 16 deviation: computed once here (rather than re-read separately
    // by the hook call below and the `VcRecord` construction further down)
    // so both timestamps are identically sourced — see `PreWriteHook`'s own
    // doc comment for why the hook needs this at all.
    let at = vc.current_time.get();
    hook.before_write(cell, loc, &old, &new, at);
    let changed = four_state_changed(&old, &new);
    if changed {
        write_slot(arena, loc, &new);
    }
    if changed && vc.record_changes.get() {
        let record = VcRecord {
            cell,
            old: old.clone(),
            new: new.clone(),
            at,
        };
        let buf_ptr = thread_local_buffer(vc);
        // SAFETY: `buf_ptr` was derived (and cached) by `thread_local_buffer`
        // from this same `vc`'s own `worker_buffers` boxed entry; per this
        // plan's own WOCB-barrier precondition (documented on
        // `VcState::register_worker_thread`), no other thread borrows this
        // buffer concurrently with a `commit_write_at` call, and this
        // function itself has not borrowed it before this line, so this is
        // the only live borrow.
        unsafe {
            (*buf_ptr).borrow_mut().push(record);
        }
    }
    ChangeResult { changed, old, new }
}

/// Task 13 — 2-state build-mode arena and read/write family (ledger ST-100,
/// ST-101, ST-102, ST-139).
///
/// A structurally separate function family from `commit_write`/`read_slot`/
/// `write_slot` above: plain `u64`, no mask half, no `FourStateValue`
/// anywhere in these signatures, and no shared code path with the 4-state
/// family except `Arena` itself (ST-139) -- which build-mode-specific
/// function family a compiled kernel calls is fixed once at CODEGEN-compile
/// time by which family CODEGEN emits calls to, never a runtime branch
/// inside either family (ST-100, ST-101).
///
/// **Deviation from the plan's literal `&mut Arena` signatures for
/// `write_inline_2s`/`write_wide_2s`:** Task 7 r1's `Arena` review fix (see
/// `state::arena`'s module doc comment) made every growing/writing `Arena`
/// accessor `&self`-based specifically so forming a fresh `&mut Arena` never
/// invalidates a previously-minted `InstanceHandle`'s raw pointer under
/// Stacked/Tree Borrows; `read_slot`/`write_slot`/`commit_write_at` above
/// already follow this discipline (`&Arena`, never `&mut Arena`). This
/// family follows the same already-established pattern for the identical
/// reason -- a `&mut Arena` parameter here would be the one Arena-mutating
/// call path in this crate that still forces a fresh `&mut Arena` into
/// existence, reintroducing exactly the hazard Task 7 r1 fixed. The
/// "changed" `bool` return value the plan's own prose describes is kept
/// exactly as specified.
///
/// Reads the current 2-state word at `word_offset`, no diffing.
pub fn read_inline_2s(arena: &Arena, word_offset: u32) -> u64 {
    arena.inline_2s()[word_offset as usize]
}

/// Writes `value` at `word_offset` iff it differs from the currently-stored
/// value (plain `u64` equality -- ST-102's "without any mask comparison":
/// there is no mask half in this family to compare). Returns whether a real
/// change occurred.
pub fn write_inline_2s(arena: &Arena, word_offset: u32, value: u64) -> bool {
    let idx = word_offset as usize;
    let old = arena.inline_2s()[idx];
    let changed = old != value;
    if changed {
        // SAFETY: every caller of this crate's write primitives upholds the
        // single-logical-writer discipline `Arena`'s own mutating accessors
        // document (see `state::arena`'s module doc comment); nothing in
        // this crate holds a live `&[u64]`/`&mut [u64]` borrow of the
        // `inline_2s` plane across this call.
        unsafe {
            arena.inline_2s_mut()[idx] = value;
        }
    }
    changed
}

/// Reads `elem_word_count` words starting at `word_offset` from the wide
/// 2-state plane, no diffing.
pub fn read_wide_2s(arena: &Arena, word_offset: u32, elem_word_count: u32) -> &[u64] {
    let start = word_offset as usize;
    let end = start + elem_word_count as usize;
    &arena.wide_2s()[start..end]
}

/// Writes `values` starting at `word_offset` in the wide 2-state plane iff
/// they differ from what's currently stored (element-wise `u64` equality,
/// no mask half). Returns whether a real change occurred.
pub fn write_wide_2s(arena: &Arena, word_offset: u32, values: &[u64]) -> bool {
    let start = word_offset as usize;
    let end = start + values.len();
    let changed = arena.wide_2s()[start..end] != *values;
    if changed {
        // SAFETY: same single-logical-writer argument as `write_inline_2s`
        // above, applied to the wide 2-state plane.
        unsafe {
            arena.wide_2s_mut()[start..end].copy_from_slice(values);
        }
    }
    changed
}

impl StateEngine {
    /// `StateCellId` allocation (ST-109): appends `loc` to a private
    /// `Vec<SlotLocation>` and returns its dense index as the new
    /// `StateCellId`, so `StateCellId(i)` and `cells[i]` are always in
    /// lockstep (no reuse/free in v1, matching the arena's own bump-allocator
    /// discipline).
    pub fn alloc_cell(&mut self, loc: SlotLocation) -> StateCellId {
        let id = self.cells.len() as u64;
        self.cells.push(loc);
        // Task 16 deviation (not in that task's own Files list, `state/
        // rollback.rs` (new) and `state/mod.rs`): `cell_reset` must stay in
        // exact lockstep with `cells` regardless of which call site
        // allocates a cell (`allocate_instance`'s real template slots,
        // `force_driver`'s synthetic contribution cells, or this crate's own
        // test helpers) — pushing the conservative `CellReset::AllX` default
        // here, unconditionally, is what guarantees that invariant; the one
        // call site that actually knows a slot's real `initial`/`net_info`
        // (`instance::StateEngine::allocate_instance`) overwrites it
        // afterwards via a direct `self.cell_reset[id] = ...` write.
        self.cell_reset.push(super::rollback::CellReset::AllX);
        StateCellId(id)
    }

    /// The one write primitive (ST-109, ST-079, ST-082, ST-091, ST-155):
    /// looks up `cell`'s already-allocated `SlotLocation` and forwards to
    /// [`commit_write_at`] — see that function's doc comment for the full
    /// algorithm. Task 12 (ST-075): on a real change, also makes exactly one
    /// call to `Dispatch::notify_scheduler` — the *only* call site
    /// `notify_scheduler` has anywhere in this crate — supplying `self.vc`'s
    /// own `current_time`/`current_region` (this plan's header "how STATE
    /// learns now" decision; never a parameter this method's own signature
    /// carries). `Dispatch::notify_waveform_and_vpi` is deliberately **not**
    /// called here: Waveform/VPI observe only through
    /// `StateEngine::drain_committed_changes` (`state::gvt`), reading the
    /// `VcRecord` [`commit_write_at`] already appended above. Scheduler
    /// wakeup is never GVT-gated — see `Dispatch::notify_scheduler`'s own
    /// doc comment for why.
    pub fn commit_write(
        &mut self,
        cell: StateCellId,
        new: FourStateValue,
        hook: &mut dyn PreWriteHook,
    ) -> ChangeResult {
        let loc = self.cells[cell.0 as usize];
        let result = commit_write_at(&self.arena, &self.vc, loc, cell, new, hook);
        self.vc.views.note_change(cell, None, &result);
        if result.changed {
            self.dispatch.notify_scheduler(
                cell,
                EdgeKind::None,
                self.vc.current_time(),
                self.vc.current_region(),
            );
        }
        result
    }

    /// The view cell of `window` of `root` (`state::alias`): a cell that
    /// owns no storage, whose value is that window of `root`'s. One cell per
    /// distinct `(root, window)`. Never reset (`root` is).
    pub fn alloc_view_cell(&mut self, root: StateCellId, window: AliasWindow) -> StateCellId {
        debug_assert!(
            self.vc.views.info(root).is_none(),
            "a window alias's root is a real cell, never another view"
        );
        let existing = self.vc.views.table().existing(root, window);
        if let Some(view) = existing {
            return view;
        }
        let loc = self.cells[root.0 as usize];
        let view = self.alloc_cell(loc);
        self.cell_reset[view.0 as usize] = super::rollback::CellReset::ForceContribution;
        self.vc.views.table().register(view, root, window);
        self.vc.views.mark_any();
        view
    }

    /// Task 13 (ST-101): read-only accessor so a test (or, later, RUNTIME)
    /// can confirm which mode a given `StateEngine` was constructed in. No
    /// `set_build_mode` method exists anywhere in this crate's public API --
    /// `BuildMode` is fixed once at `Arena::new` (via
    /// `StateEngine::new_with_mode`, `state::instance`) and never mutated
    /// after, which is the structural enforcement of "no design or
    /// implementation makes 4-state and 2-state arenas coexist at runtime".
    pub fn build_mode(&self) -> BuildMode {
        self.arena.mode()
    }
}

/// A `#[cfg(test)]`-visible peek at the *calling thread's own* registered
/// buffer's most recent entry for `cell` (single-threaded in every test that
/// uses it in this task). No production code should ever need to *query*
/// the buffer this way -- a later `drain_committed_changes` owns draining it
/// for real.
#[cfg(test)]
pub fn last_vc_record_for(cell: StateCellId) -> Option<VcRecord> {
    WORKER_CACHE.with(|cache| {
        let buffer_ptr = cache.borrow().as_ref().map(|entry| entry.buffer)?;
        // SAFETY: `buffer_ptr` was derived (and is kept alive) exactly as
        // `commit_write`'s own SAFETY comment above argues; this function
        // only ever runs on the same thread that populated the cache entry,
        // single-threaded in every test that calls it.
        let buffer = unsafe { &*buffer_ptr };
        buffer
            .borrow()
            .iter()
            .rev()
            .find(|record| record.cell == cell)
            .cloned()
    })
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::state::instance::TemplateRegistry;
    use crate::state::{ListenerRole, SignalAccess, ValueChangeSink};
    use std::sync::{Arc, Mutex};
    use vj_abi::Region;

    fn fresh_engine() -> StateEngine {
        StateEngine::new(TemplateRegistry::new())
    }

    /// Records every `notify_change` call it receives into a shared log
    /// (mirrors `state::gvt::tests::SpySink`, duplicated locally per this
    /// crate's existing per-task-module test helper convention).
    struct SpySink {
        log: Arc<Mutex<Vec<StateCellId>>>,
    }

    impl ValueChangeSink for SpySink {
        fn notify_change(
            &mut self,
            cell: StateCellId,
            _edge: EdgeKind,
            _at: SimTime,
            _region: Region,
        ) {
            self.log.lock().unwrap().push(cell);
        }
    }

    /// Allocates one inline cell (a real, valid `word_offset` into a fresh
    /// engine's own arena) and returns `(engine, cell, loc)`.
    fn inline_cell(engine: &mut StateEngine) -> (StateCellId, SlotLocation) {
        // SAFETY: `engine` is a local, single-owned value with no other live
        // borrow of the inline plane, and this single-threaded test performs
        // no concurrent allocation.
        let word_offset = unsafe { engine.arena.alloc_inline_word() };
        let loc = SlotLocation {
            class: SlotClass::Inline,
            word_offset,
            bit_width: 8,
            elem_word_count: 1,
            row_stride_words: 0,
            num_rows: 0,
        };
        let cell = engine.alloc_cell(loc);
        (cell, loc)
    }

    /// Allocates one wide cell of `elem_word_count` words.
    fn wide_cell(engine: &mut StateEngine, elem_word_count: u32) -> (StateCellId, SlotLocation) {
        // SAFETY: same reasoning as `inline_cell` above, applied to the wide
        // planes.
        let word_offset = unsafe { engine.arena.alloc_wide_words(elem_word_count) };
        let loc = SlotLocation {
            class: SlotClass::Wide,
            word_offset,
            bit_width: elem_word_count * 64,
            elem_word_count,
            row_stride_words: 0,
            num_rows: 0,
        };
        let cell = engine.alloc_cell(loc);
        (cell, loc)
    }

    fn narrow_word(val: u64, unk: u64) -> u128 {
        (val as u128) | ((unk as u128) << 64)
    }

    /// ST-109: `alloc_cell` 3x; the returned ids are dense and in lockstep
    /// with the order allocated, never reused/coalesced.
    #[test]
    fn alloc_cell_returns_dense_lockstep_indices() {
        let mut engine = fresh_engine();
        let (c0, _) = inline_cell(&mut engine);
        let (c1, _) = inline_cell(&mut engine);
        let (c2, _) = inline_cell(&mut engine);
        assert_eq!(c0, StateCellId(0));
        assert_eq!(c1, StateCellId(1));
        assert_eq!(c2, StateCellId(2));
    }

    /// ST-082: a write that does not change the already-held value produces
    /// zero VC records. The cell is pre-seeded to `Narrow(5)` via a *real*
    /// `commit_write` call (which itself appends exactly one record, going
    /// from the zero-initialized default to 5); the second, no-op
    /// `commit_write` with the same value must append nothing, leaving
    /// `last_vc_record_for` reflecting only the first write. Fails on a
    /// primitive that stores-then-diffs (always "changing" because it
    /// compares the just-written value against itself) or that skips the
    /// compare entirely (always reporting `changed: true`).
    #[test]
    fn commit_write_on_unchanged_value_produces_zero_vc_records() {
        let mut engine = fresh_engine();
        let (cell, _) = inline_cell(&mut engine);

        let seed = engine.commit_write(cell, FourStateValue::Narrow(narrow_word(5, 0)), &mut ());
        assert!(
            seed.changed,
            "seeding from the zero default to 5 is a real change"
        );

        let repeat = engine.commit_write(cell, FourStateValue::Narrow(narrow_word(5, 0)), &mut ());
        assert!(
            !repeat.changed,
            "writing the already-held value must not report a change"
        );

        let last = last_vc_record_for(cell).expect("the seed write appended a record");
        match last.old {
            FourStateValue::Narrow(w) => assert_eq!(w, narrow_word(0, 0)),
            FourStateValue::Wide { .. } => panic!("expected Narrow"),
        }
        match last.new {
            FourStateValue::Narrow(w) => assert_eq!(w, narrow_word(5, 0)),
            FourStateValue::Wide { .. } => panic!("expected Narrow"),
        }
    }

    /// Mutation regression (survived the mutation-testing pass): Task 12's
    /// (ST-075) documented guarantee that `StateEngine::commit_write` calls
    /// `Dispatch::notify_scheduler` "on a real change" -- guarded by `if
    /// result.changed`, never unconditionally -- was previously asserted
    /// only in this method's own doc comment, with no test exercising the
    /// no-op-write case through a real registered `Scheduler` sink. Seeds
    /// the cell to `5` (a real change, one notification), then repeats the
    /// identical `commit_write(cell, Narrow(5))` (no change): the spy's log
    /// must still hold exactly the one notification from the seed write.
    /// Fails if `commit_write` notified the scheduler on every call
    /// regardless of `result.changed`.
    #[test]
    fn commit_write_notifies_the_scheduler_only_on_a_real_change_never_on_a_no_op_write() {
        let mut engine = fresh_engine();
        let (cell, _) = inline_cell(&mut engine);

        let log = Arc::new(Mutex::new(Vec::new()));
        engine.subscribe(
            cell,
            ListenerRole::Scheduler,
            Box::new(SpySink { log: log.clone() }),
        );

        let seed = engine.commit_write(cell, FourStateValue::Narrow(narrow_word(5, 0)), &mut ());
        assert!(
            seed.changed,
            "seeding from the zero default to 5 is a real change"
        );
        assert_eq!(
            log.lock().unwrap().len(),
            1,
            "the seed write is a real change and must notify the scheduler exactly once"
        );

        let repeat = engine.commit_write(cell, FourStateValue::Narrow(narrow_word(5, 0)), &mut ());
        assert!(
            !repeat.changed,
            "writing the already-held value must not report a change"
        );
        assert_eq!(
            log.lock().unwrap().len(),
            1,
            "a no-op write (no real value change) must never notify the scheduler again"
        );
    }

    /// ST-082, ST-079: a write that does change the value produces exactly
    /// one VC record for that write. The cell is pre-seeded to `Narrow(5)`
    /// via a direct, low-level arena write (bypassing `commit_write`
    /// entirely, exactly the pattern Task 7's own tests use), so this test's
    /// one real `commit_write(cell, Narrow(6))` call is the *only* thing
    /// that can append to the counted buffer -- excluding the seed from the
    /// count by construction, not by coincidence.
    #[test]
    fn commit_write_on_changed_value_produces_exactly_one_vc_record() {
        let mut engine = fresh_engine();
        let (cell, loc) = inline_cell(&mut engine);

        // SAFETY: local, single-owned engine; no other live borrow of the
        // inline plane is held across this write, and this single-threaded
        // test performs no concurrent write. Bypasses `commit_write` on
        // purpose -- see this test's doc comment.
        unsafe {
            engine.arena.inline_mut()[loc.word_offset as usize] = narrow_word(5, 0);
        }

        let result = engine.commit_write(cell, FourStateValue::Narrow(narrow_word(6, 0)), &mut ());
        assert!(result.changed);
        match result.old {
            FourStateValue::Narrow(w) => assert_eq!(w, narrow_word(5, 0)),
            FourStateValue::Wide { .. } => panic!("expected Narrow"),
        }
        match result.new {
            FourStateValue::Narrow(w) => assert_eq!(w, narrow_word(6, 0)),
            FourStateValue::Wide { .. } => panic!("expected Narrow"),
        }

        let count = WORKER_CACHE.with(|cache| {
            let buffer_ptr = cache.borrow().as_ref().unwrap().buffer;
            // SAFETY: see `last_vc_record_for`'s identical SAFETY comment.
            let buffer = unsafe { &*buffer_ptr };
            buffer.borrow().iter().filter(|r| r.cell == cell).count()
        });
        assert_eq!(count, 1, "the bypassed seed write must not be counted");
    }

    /// ST-155 (the literal debugger.md-flagged edge case): a driven-`1`
    /// narrow cell (`val=1,unk=0`) written to driven-`Z` (`val=1,unk=1` per
    /// `Bit4::to_val_unk` -- value bit unchanged, mask bit flips) must
    /// report `changed: true`. A value-only comparison (`old as u64 == new
    /// as u64`, ignoring the high 64 bits) passes this exact transition
    /// unchanged and would fail this test.
    #[test]
    fn commit_write_diffs_the_mask_half_not_just_the_value_half() {
        let mut engine = fresh_engine();
        let (cell, loc) = inline_cell(&mut engine);
        // SAFETY: same as the previous test's seed write.
        unsafe {
            engine.arena.inline_mut()[loc.word_offset as usize] = narrow_word(1, 0);
        }

        let result = engine.commit_write(cell, FourStateValue::Narrow(narrow_word(1, 1)), &mut ());
        assert!(
            result.changed,
            "a driven-1 -> driven-Z transition (value bit constant, mask bit flips) must register as changed"
        );
    }

    /// ST-155, wide-tier sibling (review fix): the same X/Z-only-transition
    /// edge case, on a `Wide{aval,bval}` cell -- `aval` unchanged, `bval`
    /// flips 0->1. A hand-rolled comparator that diffs only `aval`
    /// element-wise (forgetting `bval`) would wrongly report `changed:
    /// false` and fail this test.
    #[test]
    fn commit_write_diffs_the_mask_half_not_just_the_value_half_on_a_wide_cell() {
        let mut engine = fresh_engine();
        let (cell, loc) = wide_cell(&mut engine, 1);
        // SAFETY: local, single-owned engine; no other live borrow of the
        // wide planes across this write; bypasses `commit_write` for the
        // seed, same reasoning as the narrow seed tests above.
        unsafe {
            engine.arena.wide_aval_mut()[loc.word_offset as usize] = 1;
            engine.arena.wide_bval_mut()[loc.word_offset as usize] = 0;
        }

        let result = engine.commit_write(
            cell,
            FourStateValue::Wide {
                aval: vec![1],
                bval: vec![1],
            },
            &mut (),
        );
        assert!(
            result.changed,
            "aval unchanged / bval flips 0->1 must still register as changed on the wide tier"
        );
    }

    /// ST-091, ST-031: the hook fires exactly once per `commit_write`, and
    /// strictly *before* the store -- proven by asserting, from *inside* the
    /// hook callback itself, that the cell's currently-stored value still
    /// equals `old`.
    #[test]
    fn before_write_hook_is_called_exactly_once_per_commit_write_before_the_store() {
        struct RecordingHook {
            arena: std::rc::Rc<super::super::arena::Arena>,
            calls: Vec<(StateCellId, SlotLocation, u128, u128)>,
        }
        impl PreWriteHook for RecordingHook {
            fn before_write(
                &mut self,
                cell: StateCellId,
                loc: SlotLocation,
                old: &FourStateValue,
                new_incoming: &FourStateValue,
                _at: SimTime,
            ) {
                let current = self.arena.inline()[loc.word_offset as usize];
                let old_word = match old {
                    FourStateValue::Narrow(w) => *w,
                    FourStateValue::Wide { .. } => panic!("expected Narrow"),
                };
                assert_eq!(
                    current, old_word,
                    "the stored value must still equal `old` inside the hook -- it fires before the mutation"
                );
                let new_word = match new_incoming {
                    FourStateValue::Narrow(w) => *w,
                    FourStateValue::Wide { .. } => panic!("expected Narrow"),
                };
                self.calls.push((cell, loc, old_word, new_word));
            }
        }

        let mut engine = fresh_engine();
        let (cell, loc) = inline_cell(&mut engine);
        let mut hook = RecordingHook {
            arena: std::rc::Rc::clone(&engine.arena),
            calls: Vec::new(),
        };

        engine.commit_write(cell, FourStateValue::Narrow(narrow_word(9, 0)), &mut hook);

        assert_eq!(hook.calls.len(), 1);
        let (called_cell, called_loc, old_word, new_word) = hook.calls[0];
        assert_eq!(called_cell, cell);
        assert_eq!(called_loc, loc);
        assert_eq!(old_word, narrow_word(0, 0));
        assert_eq!(new_word, narrow_word(9, 0));
    }

    /// Review fix: two explicit `register_worker_thread` calls on one engine
    /// return distinct, increasing slots; separately, a *fresh* engine on
    /// which `commit_write` is called directly (no explicit
    /// `register_worker_thread` call first) still produces a retrievable
    /// `last_vc_record_for` entry -- proving `commit_write` self-registers
    /// on first use.
    #[test]
    fn register_worker_thread_returns_distinct_increasing_slots_and_commit_write_self_registers() {
        let mut engine = fresh_engine();
        let slot0 = engine.register_worker_thread();
        let slot1 = engine.register_worker_thread();
        assert_eq!(slot0, WorkerSlot(0));
        assert_eq!(slot1, WorkerSlot(1));

        let mut fresh = fresh_engine();
        let (cell, _) = inline_cell(&mut fresh);
        fresh.commit_write(cell, FourStateValue::Narrow(narrow_word(1, 0)), &mut ());

        let record = last_vc_record_for(cell);
        assert!(
            record.is_some(),
            "commit_write must self-register this thread against the fresh engine, not require an explicit register_worker_thread call first"
        );
    }

    // -- Task 13: 2-state build-mode arena and read/write family
    // (ST-100, ST-101, ST-102, ST-139) --

    /// ST-102: the 2-state analogue of `commit_write_on_unchanged_value_produces_zero_vc_records`'s
    /// change-detection compare, using plain `u64` equality (no mask half
    /// exists in this family to compare). Fails on a primitive that
    /// stores-then-diffs (always "changing") or that skips the compare
    /// entirely (always reporting `changed: true`), and would also fail if
    /// this family accidentally routed through the 4-state `four_state_changed`
    /// mask-aware comparator instead of a plain scalar compare.
    #[test]
    fn two_state_write_changes_are_detected_without_any_mask_comparison() {
        let arena = Arena::new(BuildMode::TwoState);
        // SAFETY: `arena` is a local, single-owned value; no other live
        // borrow of the inline_2s plane across these sequential calls.
        let off = unsafe { arena.alloc_inline_2s_words(1) };

        assert!(
            write_inline_2s(&arena, off, 5),
            "uninitialized zero -> 5 is a real change"
        );
        assert!(
            !write_inline_2s(&arena, off, 5),
            "writing the already-held value must not report a change"
        );
        assert!(write_inline_2s(&arena, off, 6), "5 -> 6 is a real change");
        assert_eq!(read_inline_2s(&arena, off), 6);
    }

    /// ST-100, ST-102: proves the FFI view's wide planes stay null in
    /// `BuildMode::TwoState` even after a real wide signal has been allocated
    /// through the full `StateEngine::allocate_instance` path (Task 2's own
    /// `ffi_view_wide_planes_null_only_in_two_state_mode` test already proved
    /// this at the empty-arena level; this test proves it still holds once a
    /// 128-bit signal has actually been allocated and written through
    /// `write_wide_2s`). Fails if the FFI view always returns the (non-null)
    /// `Vec`'s pointer regardless of mode once real allocation has occurred.
    #[test]
    fn two_state_binary_never_allocates_a_wide_bval_word() {
        use crate::netlist::{SymbolKey, TemplateKey};
        use crate::state::{SlotDescriptor, SlotOffset, StateLayout};
        use std::rc::Rc;

        let templates = TemplateRegistry::new();
        let tmpl = TemplateKey {
            definition: 0,
            params_hash: 0,
        };
        // SAFETY: `templates` is a local, single-owned value with no other
        // live borrow and no concurrent caller.
        unsafe {
            templates.register(
                tmpl,
                StateLayout {
                    slots: vec![SlotDescriptor {
                        key: SymbolKey(0),
                        offset: SlotOffset {
                            class: SlotClass::Wide,
                            word_offset: 0,
                            bit_width: 128,
                            elem_word_count: 2,
                            row_stride_words: 0,
                            num_rows: 0,
                        },
                        initial: None,
                        net_info: None,
                    }],
                    inline_words: 0,
                    wide_words: 2,
                },
            );
        }

        let mut engine = StateEngine::new_with_mode(templates, BuildMode::TwoState);
        assert_eq!(engine.build_mode(), BuildMode::TwoState);

        let handle = engine
            .allocate_instance(tmpl)
            .expect("template is registered");
        let changed = write_wide_2s(
            &engine.arena,
            handle.base.wide_base,
            &[0xAAAA_AAAA_AAAA_AAAA, 0xBBBB_BBBB_BBBB_BBBB],
        );
        assert!(
            changed,
            "writing a nonzero pattern to a fresh zero-initialized cell must report changed"
        );
        assert_eq!(
            read_wide_2s(&engine.arena, handle.base.wide_base, 2),
            &[0xAAAA_AAAA_AAAA_AAAA, 0xBBBB_BBBB_BBBB_BBBB]
        );
        drop(handle);

        let arena = Rc::get_mut(&mut engine.arena)
            .expect("no other Rc<Arena> clone outstanding after dropping the handle");
        let view = arena.ffi_view();
        assert!(view.wide_aval.is_null());
        assert!(view.wide_bval.is_null());
    }

    /// ST-139: a `grep`-free structural companion -- `write_inline_2s`/
    /// `read_inline_2s` only ever index `arena.inline_2s` (their own function
    /// bodies take no `&arena.inline`/`&arena.wide_aval`/`&arena.wide_bval`
    /// argument, so there is no code path through which they could touch the
    /// 4-state planes). This test proves it observably: after a 2-state-only
    /// write, the 4-state inline plane (grown in lockstep by
    /// `alloc_inline_2s_words`, per its own deviation note in `arena.rs`, but
    /// never written to by anything in this test) is still all-zero.
    #[test]
    fn two_state_functions_never_touch_the_4state_planes() {
        let arena = Arena::new(BuildMode::TwoState);
        // SAFETY: `arena` is a local, single-owned value; no other live
        // borrow of either plane across these sequential calls.
        let off = unsafe { arena.alloc_inline_2s_words(1) };

        write_inline_2s(&arena, off, 0xDEAD_BEEF);

        assert!(
            arena.inline().iter().all(|&w| w == 0),
            "a 2-state-only write must never touch the 4-state inline plane"
        );
    }

    /// ST-101: constructing a `StateEngine` in `FourState` mode reports
    /// `FourState`; the absence of any `set_build_mode`/`&mut
    /// BuildMode`-returning method anywhere in `StateEngine`'s public API is
    /// the structural enforcement (documented, not independently
    /// runtime-tested beyond this getter returning a plain `Copy` value, not
    /// a mutable reference).
    #[test]
    fn build_mode_is_fixed_at_construction_no_setter_exists() {
        let engine = fresh_engine();
        assert_eq!(engine.build_mode(), BuildMode::FourState);

        let two_state = StateEngine::new_with_mode(TemplateRegistry::new(), BuildMode::TwoState);
        assert_eq!(two_state.build_mode(), BuildMode::TwoState);
    }

    // -- Task 11: wide/memory word-level read-write shape (ST-137, ST-138) --

    /// ST-138: a hand-built `SlotLocation` deliberately with `row_stride_words
    /// (3) != elem_word_count (2)` -- a case the real layout builder never
    /// actually produces (it always sets them equal), but which the
    /// *documented formula* must still get right, proving `resolve_mem_row`
    /// implements state.md §3's literal formula rather than one that happens
    /// to work only because the two fields are always equal in practice.
    #[test]
    fn resolve_mem_row_computes_word_offset_plus_row_stride_times_row_index() {
        let loc = SlotLocation {
            class: SlotClass::Memory,
            word_offset: 100,
            bit_width: 32,
            elem_word_count: 2,
            row_stride_words: 3,
            num_rows: 8,
        };
        assert_eq!(resolve_mem_row(loc, 0).word_offset, 100);
        assert_eq!(resolve_mem_row(loc, 3).word_offset, 109);
        assert_eq!(resolve_mem_row(loc, 7).word_offset, 121);
    }

    /// ST-138: an out-of-bounds row index must panic loudly, never compute a
    /// bogus in-range-looking offset that silently aliases whatever the arena
    /// allocated next.
    #[test]
    #[should_panic(expected = "row index")]
    fn resolve_mem_row_panics_on_an_out_of_bounds_row_index() {
        let loc = SlotLocation {
            class: SlotClass::Memory,
            word_offset: 100,
            bit_width: 32,
            elem_word_count: 2,
            row_stride_words: 3,
            num_rows: 8,
        };
        resolve_mem_row(loc, 8);
    }

    /// ST-138: row addressing must never silently apply to a non-memory slot.
    #[test]
    #[should_panic]
    fn resolve_mem_row_rejects_a_non_memory_slot_location() {
        let loc = SlotLocation {
            class: SlotClass::Wide,
            word_offset: 0,
            bit_width: 64,
            elem_word_count: 1,
            row_stride_words: 0,
            num_rows: 0,
        };
        resolve_mem_row(loc, 0);
    }

    /// ST-137: a 200-bit (4-word) wide signal's write must compare all
    /// `ceil(200/64) == 4` words before deciding change/no-change. The second
    /// `commit_write` differs from the first ONLY in word 3 (the last word) --
    /// exactly the shape that catches a truncated/prefix-only compare (e.g. a
    /// buggy `four_state_changed` that only ever looked at word 0) while a
    /// first-word-difference case would not. A third `commit_write`,
    /// byte-identical to the second across all 4 words, must then report no
    /// change at all.
    #[test]
    fn wide_write_detects_a_change_confined_to_only_the_last_of_four_words() {
        let mut engine = fresh_engine();
        let (cell, _) = wide_cell(&mut engine, 4);

        let first = engine.commit_write(
            cell,
            FourStateValue::Wide {
                aval: vec![1, 2, 3, 4],
                bval: vec![0, 0, 0, 0],
            },
            &mut (),
        );
        assert!(
            first.changed,
            "seeding from the zero default is a real change"
        );

        // Differs from `first` ONLY in the last (4th) word.
        let second = engine.commit_write(
            cell,
            FourStateValue::Wide {
                aval: vec![1, 2, 3, 999],
                bval: vec![0, 0, 0, 0],
            },
            &mut (),
        );
        assert!(
            second.changed,
            "a difference confined to the last of 4 words must still register as changed \
             -- a truncated/prefix-only word compare would miss this"
        );

        // Byte-identical to `second` -- the no-op case, exercised across all
        // 4 words, not just word 0.
        let third = engine.commit_write(
            cell,
            FourStateValue::Wide {
                aval: vec![1, 2, 3, 999],
                bval: vec![0, 0, 0, 0],
            },
            &mut (),
        );
        assert!(
            !third.changed,
            "writing the already-held 4-word value must not report a change"
        );
    }
}