Skip to content

PR_652_FIXES_SUMMARY.md

Latest commit

 

History

History
57 lines (46 loc) · 2.63 KB

PR_652_FIXES_SUMMARY.md

File metadata and controls

57 lines (46 loc) · 2.63 KB

PR #652 CodeRabbit Fixes - Summary

All 7 Actionable Issues Fixed ✅

Issue #1: Error Propagation (perspective_instance.rs lines 2425-2457)

Problem: persist_link_diff and retry_surreal_op silently swallowed errors Fix:

  • Changed both functions to return Result<(), AnyError>
  • Updated all callsites to propagate errors using ? or .expect()
  • Functions returning () use .expect() to fail-fast
  • Functions returning Result use ? to propagate

Issue #2: Documentation (MIGRATION_REMOVAL_GUIDE.md line 112-113)

Problem: Incomplete sentence in heading Fix: Changed "Can optionally be cleaned up:" to "The following can optionally be cleaned up:"

Issue #3: Migration Safety (migration.rs lines 97-119)

Problem: Marked perspective as migrated even when errors occurred Fix: Only mark as migrated when error_count == 0, otherwise return error

Issue #4: Link Status Preservation (mod.rs lines 441-457)

Problem: Hardcoded LinkStatus::Local instead of preserving original Fix: Read link.status and use it (fallback to LinkStatus::Local if None)

Issue #5: TOCTOU Race (mod.rs lines 56-66)

Problem: Race condition in initialize_from_db allowing duplicate SurrealDB services Fix: Added atomic check-and-insert after async initialization completes

Issue #6: Unique Constraint (perspective_instance.rs lines 878-891)

Problem: SurrealDB lookups only used 3 fields instead of full unique constraint (5 fields) Fix:

  • Updated get_link to accept optional author and timestamp
  • When provided, queries using all 5 fields: source, target, predicate, author, timestamp
  • All callsites updated to pass author and timestamp from LinkExpression

Issue #7: Error Handling (surreal_service/mod.rs lines 532-576)

Problem: Overly broad error check treated any "index" error as benign Fix: Removed "index" check, only checks for "unique", "duplicate", and "already exists"

Additional Compilation Fixes

  • Fixed partial move error in import_perspective (clone link.status)
  • Fixed partial move error in migration loop (borrow &links instead of consuming)

Commits

  1. 818d756e - Issues #2, #3, #7 + docstrings
  2. 6aecb225 - Issue #4 (preserve link status)
  3. 8c65f6c7 - Issue #1 (error propagation)
  4. a9e43aff - Issue #6 (unique constraint)
  5. fa3c1734 - Issue #5 (TOCTOU race)
  6. c79a9244 - Fix partial move (link.status)
  7. 640b6e65 - Fix partial move (migration loop)

Status

  • ✅ All CodeRabbit actionable comments addressed
  • ✅ Additional docstrings added for coverage
  • 🔄 Building rust-executor to verify compilation
  • ⏳ Next: cargo fmt, run full test suite
⚡