fix: address code review feedback for Phase 1c (Sessions + Export + Interchange)
Critical Issues Fixed:
- C1: Replace block_on with await in CLI handlers (main.rs)
- Async context panic: main.rs runs in tokio context, block_on causes runtime panic
- Changed tokio::runtime::Handle::current().block_on() to async .await calls
- C2: Add byte-identical round-trip export/import verification
- Verifies nodes survive export-import cycle intact
- Uses Blake3 content hash to confirm deterministic re-export behavior
Important Issues Fixed:
- I1: Fix labels comparison logic (interchange.rs)
- Properly detect empty vs None states instead of simple equality
- Accounts for both db_node and toml_node label representations
- I2: Include status in handoff notes (session.rs)
- Updated handoff_notes generation to show task status
- Format: 'id: title [status]' for remaining tasks
- I3: Add transactional import for nodes and edges (store.rs)
- New import_nodes_and_edges() method wraps all writes in BEGIN IMMEDIATE transaction
- Uses INSERT OR IGNORE for idempotent imports
- Creates parent-child Contains edges for new nodes with parents
- I4: Optimize ADR export to avoid N+1 queries (export.rs)
- Fetch all edges once instead of per-option query
- Use HashMap for efficient in-memory lookup of chosen/rejected status
- I5: Extract duplicate session row mapping (session.rs)
- Created map_session_row() helper to eliminate 4 copies of identical logic
- Applied to get_session, get_latest_session, list_sessions, and query_nodes
Minor Issues Fixed:
- M1: Update dry_run flag handling (main.rs)
- Correctly show what would be imported without performing writes
- M2: Fix comment accuracy (interchange.rs)
- Updated comment to reflect that content hash is deterministic, not timestamps
- M3: Remove unused code (interchange_test.rs, common/mod.rs)
- Removed unused imports (GraphStore, HashMap)
- Cleaned up unused helper functions
Verification:
- All 129 tests pass (14 interchange tests, 6 session tests, 13+ in other modules)
- Clippy warnings resolved except 2 minor redundant closures in store.rs
- Round-trip test confirms export-import content preservation
- Transaction safety verified with deterministic re-export behavior
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>