txlog: address correctness, efficiency, and design review findings

[schenksj]

Summary

Addresses the findings in TXLOG_MODULE_REVIEW.md for the native/src/txlog/ module, plus a follow-up multi-agent review pass over the changes themselves.

Correctness

• read_versions_concurrent now propagates non-NotFound I/O errors (transient throttles, corrupt files) instead of silently dropping versions; auto-checkpoint aborts rather than persisting a checkpoint missing committed adds/removes (C3).
• Data skipping typed comparison returns Option<Ordering>; an unknown comparison (bare-datetime filter with no session timezone) no longer prunes live files (C4). Regression tests added.
• JNI copies the full config map so session.timezone.offset.seconds reaches the skipping layer (C5).
• read_previous_checkpoint merges the schema registry into metadata so incremental checkpoints don't drop it (C6).
• checkpoint_interval is honored as a real interval (version % N == 0), not a boolean on/off flag (C7). Unit tests added.
• Truncation guards added to Eq/Gt data skipping (C9); assorted edge fixes: negative epoch micros, purge age default, probe gap → LIST fallback, UTF-8-safe debug slice, phantom v0 checkpoint refused (C10).

Efficiency

• Post-checkpoint version actions parsed while building the snapshot are reused by list_files, eliminating a 2× GET per query on tables with recent commits (E1).
• Selective compaction collapsed into full compaction — it read every manifest and produced an identical checkpoint (E3).
• Retry jitter mixes in the PID; bounded cache registry; purge cursor removes are O(N+R) instead of O(N·R) (E4).

Design

• Structural TxLogError::NotFound variant instead of substring matching (D4).
• Deleted dead garbage_collection.rs and metrics.rs (D2/D5).
• Shared datetime parser and skip-action encoder; ISO-8601 time-range values exported to Arrow instead of dropping to null (D6).

Follow-up review fixes

Gated the never-re-added diagnostic behind the debug flag, deleted the orphaned selective-compaction helpers, de-duplicated the timestamp parse chain (now also handles date-only), and corrected stale comments. checkpoint_interval semantics were confirmed as modulo (matching Delta/Quickwit convention and the @Order(30) test's own comments); the @Order(36) test uses checkpoint_interval=1 on its post-purge append so the carry-forward assertion holds under interval semantics.

Testing

• 949 Rust lib tests pass.
• TransactionLogIntegrationTest (42) and ArrowFfiListFilesIntegrationTest (7): BUILD SUCCESS.

🤖 Generated with Claude Code

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.