Add Observation support to OpenSwiftUI

[Kyle-Ye]

Summary

• Adds comprehensive Observation framework support to OpenSwiftUI
• Implements ObservationUtil for tracking observable state changes
• Integrates with OpenAttributeGraph for reactive updates

Changes

• Added OpenObservation dependency for Observation framework compatibility
• Implemented ObservationUtil with graph mutation tracking
• Added _ThreadLocal API for thread-safe observation contexts
• Added Subgraph.removeObserver API for cleanup
• Added comprehensive test coverage with ObservationUtilTests
• Updated OpenAttributeGraph dependency to fix linking issues
• Reorganized utility files (AnyAttributeFix) to better locations

Test plan

• Run swift test to verify all tests pass
• Verify ObservationUtilTests covers core functionality
• Check that existing tests still pass with new changes
• Test integration with SwiftUI compatibility layer
• Verify no performance regressions in observation tracking

[claude]

Code Review: Add Observation support to OpenSwiftUI

I've reviewed this PR and it's a comprehensive implementation of Observation framework support. Here's my detailed feedback:

✅ Strengths

Code Quality & Architecture

• Well-structured implementation following OpenSwiftUI's established patterns
• Proper use of package access level for cross-module APIs
• Good separation of concerns with clear MARK comments
• Follows existing code style conventions (4-space indentation, proper naming)

Memory Management

• Proper use of weak references (WeakUncheckedSendable, AnyWeakAttribute) to prevent retain cycles
• Careful cleanup in ObservationGraphMutation.cancel() and apply() methods
• Thread-safe implementation using ThreadSpecific for invalidations storage

Performance Considerations

• Efficient use of @inline(__always) for frequently called utility functions
• Smart deduplication with access list merging to avoid redundant tracking
• Early returns in guard statements to minimize unnecessary work
• Proper cleanup of observers and tracking when mutations are cancelled

⚠️ Areas for Improvement

Unsafe Memory Operations (High Priority)

• Lines 29-32 in ObservationTracking._AccessList.merge(): The unsafe pointer casting assumes the internal structure of _AccessList is [ObjectIdentifier: ObservationEntry]. This is brittle and could break with OpenObservation updates.
• Recommendation: Add runtime assertions or use safer APIs if available from OpenObservation.

Error Handling

• Missing error handling in installObservationSlow (lines 139-189) when subgraph operations fail
• No validation of attribute identifier validity before creating weak references
• Recommendation: Add proper error handling and logging for debugging.

TODOs & Incomplete Features

• Lines 179, 185: Missing transaction result handling and trace events
• Line 44 in tests: One test is disabled due to memory issues
• Recommendation: File issues to track these missing features.

🔧 Minor Issues

Documentation

• Some complex functions like installObservationSlow could benefit from more detailed comments explaining the observation lifecycle
• The unsafe pointer operations need clear documentation about assumptions

Test Coverage

• Good basic test coverage, but missing edge cases:
  • Error conditions and cleanup scenarios
  • Concurrent access patterns
  • Memory pressure scenarios
• One test is disabled - should be investigated and fixed

API Consistency

• Functions _withObservation and _installObservation use leading underscores suggesting internal APIs, but they're marked package - consider if this is intentional

🔒 Security Assessment

• No security concerns identified
• Proper memory management patterns followed
• Thread-safe implementation

🧪 Integration Concerns

The change to State.swift (line 269) adding options: [] parameter looks correct but should be verified:

let signalChanged = signal.changedValue(options: [])?.changed ?? false

📋 Recommendations

1. High Priority: Address the unsafe pointer casting in merge() method
2. Medium Priority: Add comprehensive error handling throughout
3. Medium Priority: Complete the TODOs for transaction results and trace events
4. Low Priority: Investigate and fix the disabled test case
5. Low Priority: Add more comprehensive test coverage for edge cases

✅ Overall Assessment

This is a solid implementation that integrates well with OpenSwiftUI's architecture. The code follows established patterns and shows good understanding of the observation lifecycle. The main concern is the unsafe memory operations that need to be addressed before merging.

Recommendation: Approve with requested changes to address the unsafe pointer operations and error handling.

[codecov]

Codecov Report

❌ Patch coverage is 8.26446% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.45%. Comparing base (4f7888f) to head (be6ee05).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...SwiftUICore/Data/Observation/ObservationUtil.swift	8.92%	102 Missing ⚠️
Sources/OpenSwiftUI/Util/AnyAttributeFix.swift	0.00%	4 Missing ⚠️
Sources/OpenSwiftUICore/Util/AnyAttributeFix.swift	0.00%	4 Missing ⚠️
Sources/OpenSwiftUICore/Data/State/State.swift	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #486      +/-   ##
==========================================
- Coverage   24.52%   24.45%   -0.08%     
==========================================
  Files         469      470       +1     
  Lines       27094    27214     +120     
==========================================
+ Hits         6645     6655      +10     
- Misses      20449    20559     +110     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.