warriors/erga/audit/2026-10-10-rtable-owner/reviews/plan.txt

# APPROVE

Fresh, independent review of `.claude/tasks/rtable-owner/{research,plan}.md` against the included sources. No blockers. This is a PLAN verdict only — no release, rung, phase, performance or provider-soundness approval is implied, and no code was written or run.

## Verification performed (summary of what I checked, not of what the plan asserts)

- **Row domain / bounds.** `1..=2^32` is coherent: max index `u32::MAX` makes `(row as u32).to_le_bytes()` exact (today's `as u32` would truncate above that). In `shaders/probe.metal`, `idx = x % n` with `ulong n ≤ 2^32`, then `table[idx*4]` → max byte offset `(n-1)*32 < logical`, no `u64` wrap. `n*32` for `n=2^32` is 2^37 — representable in `usize`/`isize`/`i64`, so `Block::plan` is pure arithmetic plus `IOSurfaceAlignProperty`/`GetPropertyAlignment` (numeric, no allocation), and the device-max check rejects before any native request. The pure-test matrix (0/1/2^32/2^32+1/`u64::MAX`) is reachable without a huge allocation.
- **Initialized extent vs logical rows.** `request = round_up(n*32, page)`, `actual == planned ≥ request ≥ logical`; raw `write_bytes(0, block.size())` on the fresh, unpublished, unimported address precedes any Rust view or `wrap`. This is the same ordering `blake-bench/src/batch.rs::Storage::initialize` already uses and that `unimem/src/block.rs` requires (`as_bytes` E0133 contract: initialize the *entire returned extent* first). Padded rows are never generated; views cover only the prefix; padded-tail canaries are tested.
- **Worker join/error paths.** Against pinned `library/std/src/thread/scoped.rs` (compiler `59807616`): `scope` drains `num_running_threads` *before* `resume_unwind`/`panic!("a scoped thread panicked")`, and `Builder::spawn_scoped` is `io::Result`. So "spawn error after k workers started" still joins all k before return; scope-owned joining (not handle `Drop`) is correctly identified as the boundary. `threads=0` rejected before mutation; `workers = min(threads, n)` with `n ≥ 1` makes `div_ceil` divisor nonzero, killing today's divide-by-zero. Worker closures can only capture `&mut [u8]` chunks — the compiler enforces this, since `Buffer`'s raw-pointer fields make `RTable`/`Probe` `!Send`.
- **Exclusive GPU lifetime / completion.** `build_parallel(&mut self)` and `run(&mut self, &mut RTable)` make a live `bytes()`/`cpu_checksum` borrow and a dispatch mutually exclusive (E0502/E0499 witnesses). No public callback, no `Dispatch`/`GpuFuture` (so no `Drop`-dependent wait), nothing fallible or allocating between `submit` and `wait`, and only `STATUS_COMPLETED` + `error() == None` restores access — the discarded status in today's `dispatch_with_bytes` is closed. Field order `Buffer` → `Block` → borrowed `&'gpu Gpu` gives the required MTLBuffer-release-before-IOSurface-unlock order. Params: 24 bytes manually serialized matches `Params { ulong n; ulong seed; uint count; uint _pad; }`; exact `count` threads keeps `gid` in `uint`; `count==0` correctly submits nothing; `acc[0..2]` = 8 initialized bytes, LE reassembly matches `cpu_checksum`'s `from_le_bytes(row[0..8])`.
- **Provider surface.** Removing public `Buffer`/`Block`/`n`/IOSurface ID, `Probe.gpu`, caller-supplied accumulator, `read_u64`/`write_u64` and public `lcg_index` closes every route by which a caller reaches `Buffer::as_bytes`/`read`/`write`/`contents_ptr` or `buffer_wrap` with an arbitrary extent. The remaining safe-but-contract-bearing calls (`Buffer::write` on the private accumulator, `Block::as_bytes_mut` on the prefix) are confined to `&mut self` paths with no in-flight work, with runtime alignment/length validation. The unit discharges its own obligations without claiming the provider globally repaired — correctly stated.
- **Reverse consumers.** `caller-inventory.json` plus the full `mine-bench` bodies confirm `mine` is the only dependent: it will break at `r_table.n` (257, 307, 444, 459, 501), `r_table.buffer`/`rb.buffer` (272/274/285), `table.block.as_bytes()` (306, 443, 269, 444, 109), `RTable::open`/`build_parallel` (45/52), and `Probe::open(Gpu)`. `ROW_BYTES` is retained, which `mine` also uses. These are intentional and reserved.
- **Not a cul-de-sac.** `RTable<'gpu>` is a plain owned struct over a borrowed device, so a future mine owner can take it by value; private fields can later be complemented by *additive* methods (consuming constructor, or an `unsafe` exact-extent import) without reopening fields. Deferring the accessor is the right order.
- **Scope hygiene.** No manifest/lock/version/provider/shader/mine/chime change; `specs/README.md` §5 replacement preserves "CPU and GPU access must address the same physical pages" verbatim and keeps observations out of the spec; ≤500-line sources; gates are named as diagnostics, not as green claims (`check -p rtable-bench` can be green because `mine-bench` is not its target; `erga-miner` does not depend on `rtable-bench`, so the bounded miner/difftest gates are reachable).

## Optional improvements (not blockers)

1. The `catch_unwind` around the scope will need `AssertUnwindSafe` (the closure captures `&mut self`-derived chunks). State once why asserting is sound: unavailability is set *before* any work and is never cleared on the error path.
2. You are already adding a private seam for the injected worker panic. Injecting a simulated `spawn_scoped` failure through the same seam (fail worker *k* of *m*) would turn failure mode (2) from source-proof into an executed witness of "started workers finish, access not restored", at near-zero cost. Keep the real-OS-failure limitation stated either way.
3. Audit wording: say plainly that the four baseline diagnostics (`rs/rtable-bench/src/lib.rs` 58/80/80/107, 3×E0133 + 1×E0596) are *discharged by this unit*, that the workspace's first error consequently moves into `mine-bench` for API reasons, and that chime stays masked behind it. Without that sentence a reader could take the new failures as "unrelated" or the baseline as "workspace-fixed".
4. Research §"Mine follow-up": importing via "the logical slice's pointer" is imprecise — the logical length `n*32` is not page-aligned, so `buffer_wrap`'s contract needs the *actual* page-aligned extent. Say that the follow-up therefore requires a new additive accessor or a narrowed provider contract, so the projection is not later read as already expressible.
5. Guard against a vacuous oracle: an unbuilt table yields CPU checksum 0 and probe 0 (the Go oracle's `reads=0` rows agree). Assert that explicitly in tests and document that only a successful build establishes hash semantics, so a future caller cannot report PASS over an all-zero table.
6. `import_memory.rs` now exists in two crates. Record it in the audit as a known duplication of safety-critical native preflight with a named future consolidation owner; divergence between the copies is the realistic decay path.
7. Once the `AtomicU64` marker goes, `acpu` and `blake2` become visibly unused direct dependencies of `rtable-bench`. The manifest is frozen, so mention them rather than deleting.
8. Consider `run(&mut self, table: &mut RTable<'gpu>)` so the device lifetime is tied statically as well; the runtime `as_raw()` identity check remains the only real guard on single-device hardware (`MTLCreateSystemDefaultDevice` returns the same object across `Gpu::open()` calls), and tying the lifetime costs nothing.
9. `rows == 0 → Mem(ZeroSize)` while `rows > 2^32 → RowCountOutOfRange` is asymmetric. Document the mapping on the variant so callers do not match `RowCountOutOfRange` expecting it to cover zero.

Root retains the audit/`decision` label, commit, PR and merge; red workspace gates and the spec change require that explicit decision.