Connection/transaction lifecycle bugs in KuzuBackend: wedged COPY retries + silently non-atomic writes #67

Summary

Two real bugs in the Kùzu local backend share one root cause: GraphStore hands out bare Connection values and leaves every caller to manage connection lifetime and transaction boundaries by hand. Some callers reuse one connection across independent statements that never needed to share state (wedging a later, unrelated write); another set of callers believes it's inside a real transaction when it isn't, because of how raw_query handles transaction-control statements.

Bug 1: connection reuse wedges a later, unrelated COPY

Observed: a COPY Symbol bulk load retries a few times after bad-primary-key errors (expected — drops the offending rows, retries). Immediately after, on the same connection, a COPY CALLS bulk load fails on its first attempt with Kùzu's internal Invalid transaction type to rollback. and falls back to the slower per-row UNWIND path. Self-healing (UNWIND produces correct data), but real and reproducible.

Root cause: import_scip_index (crates/infigraph-core/src/scip/mod.rs) opens one Connection at the top of the function and reuses it across an inline Symbol-node COPY block, then copy_edges_with_bad_record_retry (crates/infigraph-core/src/graph/store_util.rs) for CALLS edges, then again for INHERITS edges. None of these three bulk loads are wrapped in an explicit BEGIN TRANSACTION — each COPY already auto-commits independently regardless of connection sharing, so sharing the connection buys zero atomicity here while creating exposure to whatever internal state a caught COPY failure leaves behind. Same helper is also called from resolve/calls.rs and store_parquet.rs's full-reindex bulk path.

Proposed fix: copy_edges_with_bad_record_retry should take &GraphStore instead of &Connection, calling store.connection()? fresh and unconditionally on every retry-loop iteration and before the UNWIND fallback — no "is this a retry" bookkeeping needed, since GraphStore::connection() already mints a fresh Connection::new(&self.db) cheaply on every call. The inline Symbol-COPY block in import_scip_index needs the same treatment.

Bug 2: raw_query's transaction no-op silently breaks real atomicity elsewhere

KuzuBackend::raw_query opens a fresh connection per call, so it deliberately no-ops BEGIN TRANSACTION/BEGIN/COMMIT/ROLLBACK when passed as a query string (intentional — fails safe rather than erroring on a stray COMMIT). Two call sites don't know this and rely on it for real atomicity:

• concerns/mod.rs::write_concerns: BEGIN TRANSACTION, DETACH DELETE all existing Concern nodes, loop CREATE-ing new ones, COMMIT — believing this is one atomic unit.
• reflection/mod.rs: same shape.

Because raw_query's BEGIN/COMMIT are no-ops, every statement in these loops auto-commits independently and immediately. A crash mid-loop today already means the old data is gone and only part of the new data landed — live data-loss exposure, currently masked because the no-op returns Ok instead of erroring loudly.

Proposed fix: introduce GraphStore::transaction<T>(&self, f: impl FnOnce(&Connection) -> Result<T>) -> Result<T> — opens one connection, issues a real BEGIN TRANSACTION, runs the closure against that connection, commits on Ok, rolls back on Err. Migrate write_concerns, reflection/mod.rs's equivalent, KuzuBackend::write_calls_service_edges (already correct today but duplicates the boilerplate), and the two hand-rolled transaction blocks in KuzuBackend's bulk-index path onto it.

Suggested tests

• Force a bad-PK retry on one table's COPY immediately followed by a COPY on a different table on what would previously have been the same connection — assert the second COPY succeeds cleanly.
• Inject a mid-loop failure into write_concerns and assert old Concern nodes are still present afterward (this should fail against current code, proving the live bug).
• transaction() itself: commit-on-success, rollback-on-error, and no stuck BEGIN on panic.

Happy to open a PR for this if useful.