Cleanups in the handle table, membership index and tags

[Xof]

This issue groups 4 related findings.

HANDLES-INDEX-4 — handle_table.rs claims a lazily created sparse child needs "no further copy"; the recursion immediately copies it and queues it as superseded

Location: src/handle_table.rs:591, src/membership_index.rs:284 · Severity: SMELL · Category: comment-accuracy

What the code does. handle_table.rs:591-595 says: "Sparse allocation: interior pages don't pre-populate children. A zero pointer means 'no subtree allocated here yet'; we lazily create one only when an insert touches that range. The newly allocated child is already a fresh page, so it IS its own COW clone — no further copy needed." The code does the opposite: the fresh page (leaf at :598 or interior at :608) is passed as actual_child into self.insert_recursive(cache, actual_child, ...) at :622, whose first statements are let new_page = alloc(cache)?; (:552), a full PAGE_SIZE copy (:557-561), and freed.push(page_id); (:567) — so the just-created page is copied to a second fresh page and immediately pushed onto the superseded list. The identical code in membership_index.rs:284-286 documents it correctly: "A fresh child here is immediately re-COWed by the recursive call below (it becomes that frame's superseded page and is pushed to freed there); benign, first-touch only."

Why it is a problem. A maintainer reasoning about freed from this comment will conclude the list contains only previously-committed pages, when in fact it contains never-committed, this-transaction pages too — precisely the distinction that matters when deciding whether a queued id is safe to hand back out or whether a rollback must un-queue it. It also hides that each sparse-child touch costs two page allocations and an 8 KB copy instead of one.

Direction of a fix. Replace the last sentence with the membership_index.rs:284-286 wording, which describes what actually happens; or skip the redundant COW by having the fresh-child branch write the entry directly when level == 1.

HANDLES-INDEX-5 — ARCHITECTURE.md says handle-table COW pages always extend via cache.new_page; they have gone through the freemap-aware allocator since the alloc-closure refactor

Location: ARCHITECTURE.md:597, src/transaction/staging.rs:97, src/transaction/freemap.rs:638, src/handle_table.rs:26 · Severity: SMELL · Category: docs-vs-reality · Status: REGRESSION of I118 (duplicate)

What the code does. ARCHITECTURE.md:597 states: "Overflow pages and handle-table COW pages do not go through allocate_data_page (they call cache.new_page directly and always extend), but their frees still feed the freemap on commit". Every production handle-table mutation instead receives a freemap-aware closure: staging.rs:97 let mut alloc = |c: &mut PageCache| self.freemap.cow_alloc_into(c, &mut tree, reuse); immediately before self.handle_table.insert(...), the same at freemap.rs:638 for ht_insert and mutate.rs:198 for handle_table.delete. handle_table.rs:26-31 says so explicitly: "Page allocation goes through an alloc closure the caller injects (the transaction layer's freemap-aware cow_alloc), so superseded pages from a prior committed transaction are reused before the file is extended." The only remaining cache.new_page() in the module is create_root (handle_table.rs:175), which runs once per database.

Why it is a problem. The doc understates reclamation for the handle table and, read alongside the freemap chapter, tells a maintainer that handle-table growth is extend-only — leading to wrong conclusions about steady-state file size and about which allocations are affected by the savepoint reuse carve-out (reuse = self.savepoints.is_empty(), staging.rs:93), a nuance the doc does not mention at all for these pages.

Direction of a fix. Rewrite ARCHITECTURE.md:597 to say handle-table and membership-index COW pages allocate through cow_alloc (freemap reuse first, extend as fallback, reuse disabled while a savepoint is open), and that only create_root and overflow pages extend directly.

HANDLES-INDEX-6 — Chisel::tag and Chisel::handles_with_tag doc-comments still describe the pre-newtype u32 API, including a Tag 0 case the type makes unconstructable

Location: src/lib.rs:614, src/lib.rs:644 · Severity: SMELL · Category: comment-accuracy

What the code does. src/lib.rs:614 documents tag as "Returns 0 for untagged handles", but the signature two lines down (:618) is pub fn tag(&self, handle: Handle) -> Result<Option<Tag>> and the body is self.txm.tag(handle.get()).map(Tag::new) // stored 0 -> None — untagged yields None, never 0, and Tag cannot hold 0 at all (Tag(NonZeroU32), handle.rs:94). src/lib.rs:644 documents handles_with_tag with "Tag 0 always returns an empty Vec (the membership index is not updated for untagged values)", but its parameter is tag: Tag (:655), so the described call cannot be written — Tag::new(0) returns None (handle.rs:100-105) and Tag::try_from(0) returns Err(ZeroTagError) (handle.rs:128-133).

Why it is a problem. These are the two doc blocks a user reads to learn how "untagged" is represented at the public boundary, and they contradict the exact design decision handle.rs:88-92 was written to make legible ("'No tag' is the ABSENCE of a Tag (Option<Tag>) — Tag(0) is unconstructable"). A reader following lib.rs:614 will write if db.tag(h)? == 0, which does not compile, and will look for a zero-tag branch that cannot exist.

Direction of a fix. Change lib.rs:614 to "Returns None for untagged handles" and delete the "Tag 0 always returns an empty Vec" sentence at lib.rs:644 (the untagged case is now unrepresentable in the signature).

HANDLES-INDEX-7 — MAX_DEPTH is enforced only on the recovery walk, not on the growth loop, which does not terminate for handle == u64::MAX

Location: src/handle_table.rs:253, src/handle_table.rs:77, src/membership_index.rs:243 · Severity: SMELL · Category: correctness · Status: KNOWN as I147

What the code does. The header asserts a hard bound: handle_table.rs:75-81 — "capacity(5) ≈ 5.7e17 < u64::MAX < capacity(6), so a tree keyed by u64 handles is never deeper than 6 (a handle in (capacity(5), u64::MAX] forces one final grow to depth 6, whose capacity saturates to u64::MAX). Any spine claiming a deeper tree is corrupt". MAX_DEPTH is referenced only inside recover_depth (:448). The growth loop is while handle >= self.capacity() { current_root = self.grow(...)?; } (:253) and capacity() uses saturating_mul (:483), so at depth >= 6 it returns u64::MAX. For handle == u64::MAX the predicate u64::MAX >= u64::MAX stays true after every grow, so the loop keeps calling grow — allocating one page and incrementing self.depth per iteration — until alloc errors. RadixU64::insert has the identical loop at membership_index.rs:243. Note that find_leaf explicitly reasons about this saturation case (handle_table.rs:688-693, the cap != u64::MAX clause) while the growth loop does not.

Why it is a problem. With handle == u64::MAX the tree is driven past the depth the module documents as impossible-unless-corrupt, and the only thing that stops it is an allocation failure after having extended the file. Unreachable today — next_handle starts at 1 and is only ever += 1 (staging.rs:325), so reaching u64::MAX needs ~1.8e19 committed allocations, and no public API lets a caller name an arbitrary handle for insert. The defect is that the stated depth invariant is enforced nowhere on the write path, so the header's "never deeper than 6" is a claim about arithmetic, not about the code.

Direction of a fix. Bound the loop explicitly — while self.depth < MAX_DEPTH && handle >= self.capacity() in both radices — and let the (now impossible) leftover case fall out as a typed error rather than an unbounded grow; that also makes MAX_DEPTH a real invariant instead of a recovery-only constant.

---

_{Filed from the clean-slate deep review of 2026-07-29. Full context, verification notes, and the delta against ISSUES.md are in docs/reviews/review-20260729-183138.md. Baseline at review time: 681 tests passing, clippy and fmt clean — none of these are toolchain-visible.}