Architectural Fixes & Improvements

This document describes critical architectural fixes applied to ObjectState to resolve re-entrant cache invalidation issues and ensure robust state management.

Background

ObjectState manages configuration state with lazy resolution, caching, and automatic delegate synchronization. These features, while powerful, created architectural coupling that led to re-entrant cache invalidation during save operations.

The Problem: Re-entrant Cache Invalidation

When saving GlobalPipelineConfig, the following sequence would occur:

mark_saved() called on GlobalPipelineConfig (scope=’’)
Invalidation propagates to descendants (e.g., /home/ts/test_plate)
Descendant calls _recompute_invalid_fields() to refresh its cache
During recomputation, get_ancestor_objects_with_scopes() is called
This attempts to get the current object via to_object()
to_object() calls _check_and_sync_delegate() (delegate auto-detection)
Delegate sync calls invalidate_cache()
``_live_resolved`` is set to ``None`` while we’re actively computing it
Next .get() call crashes: 'NoneType' object has no attribute 'get'

This is a re-entrant call problem where a query operation (to_object()) has hidden mutation side effects (delegate sync → cache invalidation).

Architectural Fixes Applied

Fix 1: Separate Query from Mutation in `to_object()`

Problem: to_object() unconditionally called _check_and_sync_delegate() even though it’s conceptually a query operation.

Solution: Added sync_delegate parameter to control this behavior:

def to_object(self, *, update_delegate: bool = False, sync_delegate: bool = True):
    """Reconstruct object from flat parameters.

    Args:
        update_delegate: If True, apply reconstructed delegate to object_instance
        sync_delegate: If True (default), check for delegate changes before reconstruction.
                      Set to False during cache recomputation to avoid re-entrant invalidation.
    """
    if sync_delegate:
        self._check_and_sync_delegate()
    # ... rest of reconstruction

Impact:

Query purity: Can now call to_object(sync_delegate=False) without side effects
Backward compatible: Default behavior unchanged (sync_delegate=True)
Explicit control: Delegate sync happens when you ask for it

Fix 2: Enable Real-Time MRO Inheritance

Problem: During _recompute_invalid_fields(), the code was using self.object_instance (original saved state) instead of reconstructing from current parameters. This broke real-time MRO inheritance—editing WellFilterConfig.well_filter wouldn’t update placeholders for inheriting fields like PathPlanningConfig.well_filter.

Solution: Reconstruct current object from parameters without delegate sync:

# In _recompute_invalid_fields():

# Get ancestors without triggering delegate sync
ancestor_objects_with_scopes = ObjectStateRegistry.get_ancestor_objects_with_scopes(
    self.scope_id,
    skip_delegate_sync=True
)

# Reconstruct from CURRENT parameters for real-time inheritance
current_obj = self.to_object(update_delegate=False, sync_delegate=False)

Impact:

✅ Real-time inheritance: Editing a base config field immediately updates all inheriting fields
✅ No cache invalidation: sync_delegate=False prevents re-entrant invalidation
✅ Fresh values: Objects reconstructed from current parameters, not stale saved state

Fix 3: `skip_delegate_sync` Parameter in `get_ancestor_objects_with_scopes()`

Problem: This method was calling to_object() or saved_object on all ancestors, triggering delegate sync even when just building a context stack.

Solution: Added skip_delegate_sync parameter:

@classmethod
def get_ancestor_objects_with_scopes(
    cls,
    scope_id: Optional[str],
    use_saved: bool = False,
    skip_delegate_sync: bool = False
):
    """Get (scope_id, object) tuples from ancestors.

    Args:
        skip_delegate_sync: If True, skip delegate sync during object retrieval.
                           Use during cache recomputation to avoid re-entrant invalidation.
    """
    if use_saved:
        obj = state._extraction_target  # Direct access, no sync
    else:
        obj = state.to_object(update_delegate=False, sync_delegate=not skip_delegate_sync)

Impact:

Controlled side effects: Caller decides whether delegate sync happens
Cache safety: Can build context stacks without triggering invalidation
Performance: Avoids unnecessary delegate checks during resolution

Fix 4: Descendant Saved Baseline Propagation

Problem: When GlobalPipelineConfig was saved, descendant states (plates, steps) retained their * dirty markers because their _saved_resolved wasn’t updated to reflect the new ancestor saved values.

Solution: After mark_saved() updates _saved_parameters, propagate to descendants:

# In mark_saved():

# After updating own saved_resolved, propagate to descendants
changed_scope = ObjectStateRegistry._normalize_scope_id(self.scope_id)
if changed_scope == "":
    # Global baseline change affects ALL other states
    descendant_scopes = [
        s.scope_id for s in ObjectStateRegistry._states.values()
        if ObjectStateRegistry._normalize_scope_id(s.scope_id) != ""
    ]
else:
    prefix = changed_scope + "::"
    descendant_scopes = [
        s.scope_id for s in ObjectStateRegistry._states.values()
        if s.scope_id and ObjectStateRegistry._normalize_scope_id(s.scope_id).startswith(prefix)
    ]

# Recompute each descendant's saved_resolved with new ancestor values
for descendant_scope in descendant_scopes:
    state = ObjectStateRegistry._states.get(descendant_scope)
    if state:
        state._saved_resolved = state._compute_resolved_snapshot(use_saved=True)
        state._sync_materialized_state()  # Recalculate dirty fields

Impact:

✅ Correct dirty detection: Descendant dirty markers (*) clear when parent is saved
✅ Cascade semantics: Saving a parent propagates the saved baseline to all descendants
✅ UI consistency: Users see expected behavior when saving global config

Fix 5: Backward Compatibility for `saved_parameters=None`

Problem: Old snapshots/history could have saved_parameters=None or omit it entirely, causing 'NoneType' object has no attribute 'get' errors during restore or save.

Solution: Added defensive guards in multiple locations:

# In _compute_resolved_snapshot():
if use_saved and self._saved_parameters is None:
    logger.warning(f"_saved_parameters is None for scope={self.scope_id}, using parameters as fallback")
    self._saved_parameters = copy.deepcopy(self.parameters)

# In snapshot deserialization:
saved_parameters=(
    state_data.get('saved_parameters')
    if state_data.get('saved_parameters') is not None
    else state_data['parameters']
)

# In history import:
saved_parameters=(
    state_data.get('saved_parameters')
    if state_data.get('saved_parameters') is not None
    else state_data['parameters']
)

Impact:

✅ Robustness: Handles legacy snapshots gracefully
✅ No crashes: Defensive guards prevent AttributeError
✅ Automatic recovery: Falls back to parameters when saved_parameters is missing

Architectural Principles

These fixes embody several key architectural principles:

1. Separation of Concerns

Query operations should not have mutation side effects. The sync_delegate parameter separates the query (get current object) from the mutation (sync delegate).

2. Explicit Over Implicit

Delegate synchronization is now explicitly controlled via parameters rather than happening automatically as a hidden side effect.

3. Re-entrancy Safety

Methods that call themselves (directly or indirectly) must guard against invalidating state they’re actively computing.

4. Defensive Programming

Handle edge cases (None values, missing data) gracefully rather than crashing.

5. Backward Compatibility

New parameters default to preserving existing behavior. Existing code continues to work.

Testing & Validation

These fixes were validated through:

Manual testing: Save GlobalPipelineConfig → no crash, descendants clear dirty markers
Real-time inheritance: Edit WellFilterConfig → inheriting fields update immediately
Legacy compatibility: Old snapshots with saved_parameters=None load correctly
Multi-level inheritance: Global → Pipeline → Step inheritance works correctly

Migration Guide

For library users, no migration is required. The changes are backward compatible.

For library developers extending ObjectState:

If you call ``to_object()``:

Default behavior unchanged
To avoid delegate sync during cache operations: to_object(sync_delegate=False)

If you call ``get_ancestor_objects_with_scopes()``:

Default behavior unchanged
To avoid delegate sync during resolution: get_ancestor_objects_with_scopes(scope_id, skip_delegate_sync=True)

Performance Impact

Positive: Skipping delegate sync during cache recomputation reduces unnecessary checks
Neutral: Default behavior unchanged for normal operations
Positive: Context stack building is faster when skip_delegate_sync=True

Future Improvements

Potential future enhancements:

Remove delegate auto-sync entirely: Move to explicit sync-only at clear boundaries (save, load)
Deferred invalidation: Queue invalidations during cache computation rather than blocking them
Immutable snapshots: Make _live_resolved immutable during computation to catch bugs early

Summary

These architectural fixes resolve a critical re-entrant cache invalidation issue while maintaining backward compatibility and improving real-time MRO inheritance behavior.

Key takeaway: When designing stateful systems with caching and auto-sync features, carefully separate query operations from mutations and guard against re-entrant calls that invalidate state being actively computed.

Architectural Fixes & Improvements

Background

The Problem: Re-entrant Cache Invalidation

Architectural Fixes Applied

Fix 1: Separate Query from Mutation in to_object()

Fix 2: Enable Real-Time MRO Inheritance

Fix 3: skip_delegate_sync Parameter in get_ancestor_objects_with_scopes()

Fix 4: Descendant Saved Baseline Propagation

Fix 5: Backward Compatibility for saved_parameters=None

Architectural Principles

Testing & Validation

Migration Guide

Performance Impact

Future Improvements

Summary

Fix 1: Separate Query from Mutation in `to_object()`

Fix 3: `skip_delegate_sync` Parameter in `get_ancestor_objects_with_scopes()`

Fix 5: Backward Compatibility for `saved_parameters=None`