Cleanups in the Swift/UniFFI binding

[Xof]

This issue groups 5 related findings.

SWIFT-8 — Swift isPoisoned doc omits that a closed handle also reports true — the Rust doc it wraps says so explicitly

Location: swift/Sources/Chisel/ChiselDatabase.swift:53, swift/Sources/Chisel/ChiselStore.swift:62, chisel-ffi/src/database.rs:86 · Severity: SMELL · Category: comment-accuracy · Status: NEW

What the code does. The Rust side documents both conditions: "True if the handle can no longer do work: the engine reports is_poisoned(), OR the handle is already closed" (database.rs:86-87), implemented as guard.as_ref().map(chisel::Chisel::is_poisoned).unwrap_or(true) (database.rs:90-93) — None (taken by close) yields true. Both Swift doc comments drop the second half: "True if a fatal error has poisoned the underlying engine handle." (ChiselDatabase.swift:53) and "Whether the engine has poisoned itself (a fatal integrity/I-O failure)." (ChiselStore.swift:62-63). The Rust unit test even asserts the closed case, with a comment acknowledging the conflation: db.close().unwrap(); ... assert!(db.is_poisoned()); (database.rs:293-296).

Why it is a problem. A caller that closes cleanly and then checks health — e.g. a diagnostics screen calling store.isPoisoned() on a shut-down store, or the README's own drop-and-reopen recovery flow (swift/README.md:144-145) which calls close() first — reads true and concludes the database file is corrupt, per the documented meaning of "fatal error has poisoned". Correct code, wrong conclusion.

Direction of a fix. Copy the Rust phrasing into both Swift doc comments: true if the engine is poisoned OR the handle has been closed.

SWIFT-9 — transaction/savepoint doc says it rethrows "what body threw", but a failing commit/release is also caught, rolled back, and rethrown

Location: swift/Sources/Chisel/ChiselDatabase.swift:113, swift/Sources/Chisel/ChiselDatabase.swift:181 · Severity: SMELL · Category: comment-accuracy · Status: NEW

What the code does. The doc reads "Commits on normal return; on a thrown error, rolls back (best-effort ... swallowed via try?) and rethrows what body threw." (ChiselDatabase.swift:113-116). The code puts the commit inside the do: let result = try body(txn); try native.commit(); return result } catch { try? native.rollback(); throw error } (ChiselDatabase.swift:121-127). savepoint has the identical shape — "on a thrown error, rolls back to name ... then rethrows" (:182-183) with try native.release(name: name) inside the do (:189).

Why it is a problem. When body returns normally and commit() fails, the catch arm still runs: it issues a rollback() on an engine that has just poisoned itself on the commit path (src/lib.rs:529-530: "A failure inside the fsync/superblock protocol is fatal and poisons the handle"), and rethrows the COMMIT error — not "what body threw", since body threw nothing. A maintainer reading the comment would believe the catch arm is reachable only from body, and could add cleanup there (e.g. re-running body's compensating action, or logging "user code failed") that misfires on every commit failure.

Direction of a fix. State what the code does: the catch arm covers a throw from body OR from commit()/release(), and rethrows whichever one threw. If commit-failure should not trigger a rollback attempt, move try native.commit() outside the do.

SWIFT-10 — Workspace comment still describes three members and "excludes python" after chisel-ffi became the fourth excluded member

Location: Cargo.toml:1, Cargo.toml:21 · Severity: SMELL · Category: comment-accuracy · Status: NEW

What the code does. On design/swift-binding, members = [".", "python", "bench", "chisel-ffi"] (Cargo.toml:20) and default-members = [".", "bench"] (Cargo.toml:31), but the surrounding comments were not updated. The header still says the members "exercise the engine, the PyO3 binding, and the bench harness in one shot" (Cargo.toml:3-4) — three of four — and the default-members paragraph opens "default-members excludes python from cargo build / cargo test from the workspace root" (Cargo.toml:21-22) and then spends nine lines explaining pyo3's extension-module linker behaviour, never mentioning chisel-ffi. The consequence is spelled out elsewhere, in .github/workflows/swift.yml:33-37: "cargo test ... runs plain cargo test --verbose, which honors default-members = [\".\", \"bench\"] and excludes chisel-ffi. So chisel-ffi's Rust test suite needs its own gate".

Why it is a problem. A maintainer reading Cargo.toml concludes that only python is excluded and that a root cargo test therefore covers chisel-ffi. It does not — the 15 unit tests in chisel-ffi/src/{database,types,error}.rs (including the whole error-mapping totality guard at error.rs:347-453) run only via the single macOS-only cargo test -p chisel-ffi step in swift.yml. Anyone running the project's documented local pre-push check would ship an untested FFI crate.

Direction of a fix. Add chisel-ffi to the default-members comment with its own one-line reason (Apple-target staticlib/cdylib crate, gated by swift.yml), and update the three-member list in the header.

SWIFT-11 — PARITY.md's line references into ChiselDatabase.swift are stale for 8 rows, in a table whose header claims it was verified by reading the file

Location: swift/PARITY.md:95, swift/PARITY.md:8 · Severity: SMELL · Category: docs-vs-reality · Status: NEW

What the code does. PARITY.md:8-9 asserts the Swift column is "verified by reading each file directly, not inferred from the plan." Eight rows disagree with the file. Chisel.read → "ChiselDatabase.swift:58" (PARITY.md:95); line 58 is blank. Actual public func read is line 73. Likewise handles :65→75 (PARITY.md:99), stats :67→77 (:107), counters :69→79 (:108), tag :73→85 (:101), handles_with_tag :75→87 (:102), client_byte :79→91 (:105), get_root_name :83→95 (:110). The neighbouring rows are all correct — isPoisoned:54, close:57, fileSizeBytes:81, addKey:99, transaction:117, allocate:142, Savepoint.name:209 — as is every one of the 19 ChiselStore.swift references and all six generated-file references at PARITY.md:49-50 (819/854/996/1026/974/1005 all check out). The drift is confined to the block that shifted when the seven-line "Mutating operations ... are NOT mirrored here" comment (ChiselDatabase.swift:64-70) was inserted.

Why it is a problem. The table is the stated source of truth for Python↔Swift parity and the artifact a reviewer uses to confirm "No known gaps" (PARITY.md:16). Eight of its citations point at blank lines and mid-comment text, so the next person to audit parity either wastes time or — worse — trusts the unverifiable rows because the verified-by-reading claim in the header is still there.

Direction of a fix. Regenerate the ChiselDatabase.swift column, or drop bare line numbers in favour of symbol names, which do not rot when a comment block is inserted.

SWIFT-12 — Test comment claims a 32-bit-truncation boundary check that the chosen value cannot detect

Location: chisel-ffi/src/types.rs:327 · Severity: SMELL · Category: comment-accuracy · Status: NEW

What the code does. The comment reads: "Exercises the u64 -> usize cast in DefragOptions::from at both ends of its range: 0 (no-op limit) and a large value that only differs from its usize form if the cast silently truncated (e.g. on a 32-bit usize). The prior version of this test used max_values: 42, which isn't a boundary of anything." (types.rs:327-331). The values actually used are max_values: 0 and max_values: 1_000_000 (types.rs:337, 344), asserted against 0usize and 1_000_000usize. The cast under test is .max_values(o.max_values as usize) (types.rs:232).

Why it is a problem. 1,000,000 is roughly 0.02% of u32::MAX, so it round-trips identically through a 32-bit usize — the test cannot fail for the reason its comment gives, on any platform. Neither is it "both ends of" a u64 range. A maintainer trusting the comment believes the truncation path is covered and would not add the check when a 32-bit Apple target (or any other 32-bit consumer of chisel-ffi) is reintroduced; the first real truncation would be a silently reduced defrag budget, not an error.

Direction of a fix. Either assert the real boundary (u64::MAX / u32::MAX as u64 + 1) with the platform-appropriate expectation, or rewrite the comment to say what the test actually covers (zero and a representative non-zero value) and drop the truncation claim.

---

_{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.}