Add onDidSet closure API to Stored nodes

[muukii]

Summary

• Add onDidSet method to Stored nodes for observing value changes
• Handler receives (oldValue, newValue) and is called after value updates via wrappedValue setter
• Thread-safe implementation using existing lock mechanism
• Not triggered by external storage updates (e.g., UserDefaults changes)

Usage

let counter = Stored(wrappedValue: 0)
counter.onDidSet { oldValue, newValue in
  print("Changed from \(oldValue) to \(newValue)")
}

Test plan

• Test didSet is called on value change
• Test didSet is called for multiple changes
• Test didSet is called even when value is the same
• Test didSet handler can be replaced

🤖 Generated with Claude Code

[claude]

Claude finished @muukii's task —— View job

---

PR Review: Add onDidSet closure API to Stored nodes

Todo List:

• Read and analyze StorageAbstraction.swift changes
• Read and analyze NodeObserveTests.swift changes
• Review code quality and Swift best practices
• Check for potential bugs and edge cases
• Evaluate performance considerations
• Review security implications
• Assess test coverage and quality
• Provide comprehensive feedback

Review Summary ✅

This is a well-implemented feature that adds a clean and thread-safe onDidSet closure API to Stored nodes. The implementation follows Swift best practices and integrates seamlessly with the existing architecture.

Code Quality & Best Practices ✅

Strengths:

• Thread Safety: Proper use of existing lock mechanism (lock.lock()/lock.unlock()) around didSetHandler access in Sources/StateGraph/StorageAbstraction.swift:339-341
• Clean API Design: Simple, intuitive interface that follows Swift conventions
• Integration: Leverages existing infrastructure without duplicating code
• Memory Safety: Uses nonisolated(unsafe) appropriately for the private didSetHandler field
• Documentation: Good inline documentation explaining behavior and limitations

Implementation Details:

• Handler is called after the value is set but outside the lock in Sources/StateGraph/StorageAbstraction.swift:224
• Correctly captures the old value before modification in Sources/StateGraph/StorageAbstraction.swift:205
• Handler replacement works correctly by simply overwriting the stored closure

Security & Performance ✅

Security:

• No security concerns identified
• Proper memory management with weak references where appropriate
• Thread-safe implementation prevents data races

Performance:

• Minimal overhead: only stores a single optional closure
• Handler execution happens outside the critical lock section
• No performance impact when handler is not set (nil check is fast)

Test Coverage ✅

Excellent test coverage in Tests/StateGraphTests/NodeObserveTests.swift:151-223:

• ✅ Basic functionality (handler called with correct old/new values)
• ✅ Multiple sequential changes
• ✅ Behavior with identical values (handler still called)
• ✅ Handler replacement
• ✅ Uses Swift Testing framework appropriately

Minor Suggestions

1. Consider adding thread safety test: While the implementation is thread-safe, adding a test that verifies concurrent access patterns would be valuable:

   @Test("onDidSet is thread-safe with concurrent updates")
   func testThreadSafety() async {
     // Test concurrent wrappedValue sets with handler
   }

2. Documentation enhancement: Consider adding a code example in the method documentation showing typical usage patterns.

Edge Cases Considered ✅

The implementation handles edge cases well:

• ✅ Handler replacement (only latest handler is called)
• ✅ Same value updates (handler still called, which is correct behavior)
• ✅ No handler set (nil check prevents crashes)
• ✅ External storage updates correctly bypass the handler (as documented)

Architecture Fit ✅

This feature integrates perfectly with the existing reactive graph architecture:

• Follows the same patterns as other observation mechanisms
• Uses existing lock infrastructure
• Doesn't interfere with the existing observe() async stream API
• Clear separation of concerns (direct setter observation vs. stream-based observation)

Recommendation: ✅ APPROVE

This is a high-quality implementation that adds valuable functionality without compromising the existing codebase. The code is well-tested, thread-safe, and follows established patterns in the library.