refactor: centralize async FFI completion handling

[jja725]

Summary

Intent:

• Make future-to-callback translation reusable for additional async C APIs.
• Detect accidental layout drift between the Rust ABI and the public C header.

Changes:

• Add a generic async bridge that maps success, Lance errors, and panics to exactly one completion.
• Route asynchronous scanner materialization through the shared completion abstraction.
• Compare public enum sizes and struct layouts across compiled C and Rust definitions.

---

_{Generated by the 🪄 pr-create skill in devexp-agent-marketplace}

Test Plan

Issues

[jja725]

@zhangstar333 @u70b3 PTAL

[lance-gatekeeper]

✅ Gate recommendation: approve.

The reusable bridge preserves the established exact-once completion, callback-thread error, panic-poisoning, and Arrow stream ownership contracts while removing scanner-specific duplication. The compiled C/Rust layout comparison protects the current public Lance-owned enums and structs without changing the public API.

[zhangstar333]

catch_unwind starts inside the spawned task, so runtime initialization or RT.spawn panics are not caught by this helper.
The scanner path already has an outer guard, this is fine today; perhaps could document this requirement for future callers.

[u70b3]

Non-blocking follow-up: for the enums above, only size/align are compared — discriminant values are not. The header pins explicit values (e.g. LANCE_ERR_PANIC = 9, LANCE_INDEX_IVF_SQ = 102), so a one-sided value edit or a variant reorder on either side would still print 4/4 and pass undetected. A follow-up could print one E|Type.VARIANT|value record per enumerator (matched against as i32 on the Rust side), or _Static_assert pairs in the C probe. Not a blocker for this PR — the layout axes guarded here are already worth having; value drift can be a separate guard.

[LuciferYang]

Review from a code-review pass: 7 findings (4 MEDIUM, 3 LOW), no blockers.

The refactor itself checks out: the deleted scanner.rs block's invariants — exactly-one completion, poison-on-panic, the issue #61 dual-channel reporting, and the GuardedReader wiring — are all re-established in the new bridge, and a panicking on_panic hook is now strictly safer than before (the old code would have died as an unobserved JoinError with zero completions). CI green on Linux/macOS.

Relative to the PR's own stated goals, two halves are missing:

• Reusable for additional async C APIs — the bridge centralizes the easy template; the four things the next caller must get right (entry panic guard, dual-channel validation report, NULL-callback preamble, per-handle panic policy) still rest on caller discipline, and Completion's Copy + by-value succeed/fail means exactly-once isn't type-enforced.
• Detect ABI layout drift — the test never compares enum discriminant values (C-side implicit value collisions compile silently under the test's own -Wall -Wextra -Werror — verified), and nothing forces new public types into the coverage list (LanceSessionCacheStats from #63 is already uncovered on main).

Three smaller ones: the converter-panic path is untested, the panic in FFI call format string is duplicated in six places, and the NULL-scanner branch of scan_async has no test (pre-existing). The MEDIUM fixes are all small (drop a derive, compare sorted value lists, ~10 lines of header parsing) and would fit in this PR.

[LuciferYang]

spawn_lance_future centralizes the easy template (spawn + catch_unwind + dispatch), but the four things the next async API must get right still rest on caller discipline: the entry panic guard (scan_async_guarded), the dual-channel validation report (caller-thread set_last_error + completion.fail, the issue #61 contract), the NULL-callback preamble, and the per-handle-type panic policy the caller must choose (the scanner rejects via a poison flag; the dataset is deliberately poison-tolerant, so a no-op on_panic there would be by design — just undocumented). Missing any of these compiles fine and no test fails.

Could the guarded entry and the dual-channel reporting fold into the bridge — say a Completion::fail_validation (set TLS + dispatch) plus a guarded entry helper? At minimum, listing these caller obligations in the doc comment would make the reuse claim cover the half that matters.

[LuciferYang]

Completion derives Copy while succeed/fail take self by value, so a used completion stays callable: calling completion.fail(...) on a branch and then passing the same value into spawn_lance_future compiles, the host callback fires twice, and the exactly-once promise in lance.h is broken (potential double-free of the result).

Dropping Copy (keeping Clone) turns any second use into a use of moved value compile error — exactly-once becomes a type guarantee instead of caller discipline. Two sites then need an explicit .clone(): completion_on_panic in async_bridge.rs, and scan_async_guarded, which moves the completion into the setup closure and still needs it in the panic arm; the "copyable endpoint" doc comment should be updated too.

[LuciferYang]

The test records only size/align/offset — enum discriminant values are never compared. Adding the same variant on both sides with = 5 in Rust and = 6 in C, or inserting a C enumerator without an explicit value so it collides with the next one (legal C, no warning), keeps the test green while outgoing values (LanceErrorCode from lance_last_error_code, LancePollStatus) silently mis-map. Today all 14 enums carry explicit matching discriminants, so this is a missing defense, not a live bug.

Compare each type's sorted value lists instead — variant names don't line up (LANCE_ERR_IO maps to IoError), so name-keyed records false-fail on all 14 enums. Build the Rust list from Type::Variant as i32 path expressions so renames/removals fail to compile, like the field lists already do. Values can be negative (LancePollStatus::Error = -1), so compare signed; a sorted list can't say which variant drifted (fine for an alarm), and swapping two values on one side still passes.

[LuciferYang]

The covered set is hand-maintained in three places (header, abi_layout.c, Rust macros) and nothing forces a new public type into the test. Not hypothetical: LanceSessionCacheStats (#63, merged to main after this branch point) is outside every layout test and stays uncovered once this PR merges back. The Rust-side lists do fail at compile time on renames/removals; additions are the silent side.

Parsing include/lance/lance.h in the Rust test for named typedefs ending in } LanceFoo; (opaque handles are forward declarations and don't match) and asserting each appears as a T| record — about ten lines — turns "forgot to cover" into a hard failure.

[LuciferYang]

The four bridge tests cover success, Lance error, future panic, and a panicking on_panic hook — but not a panicking into_result, the path that leans hardest on evaluation order (argument evaluated before succeed is entered, whole call inside catch_unwind). Lose either condition and a panicking converter delivers zero completions and hangs the host.

A fifth case mirroring panic_runs_hook_and_delivers_panic_error, asserting on_panic ran and exactly one Panic completion arrived, pins this down in ~15 lines.

[LuciferYang]

The panic in FFI call: ... format string appears six times (async_bridge, scan_async_guarded, three older copies in scanner.rs, and recover_from_ffi_panic in error.rs); once one site drifts, hosts string-matching panic messages see two formats for the same class of error. Extracting an error::panic_ffi_message(&payload) helper unifies all six.

[LuciferYang]

NULL callback, poisoned scanner, setup panic, and success paths all have tests, but nobody passes a NULL scanner to lance_scanner_scan_async and asserts the dual-channel (caller-thread TLS + dispatched completion) InvalidArgument result. Pre-existing gap — and this branch is exactly the template future async APIs will copy, so this PR is the natural moment to add it.