Skip to content

Keep atomic and deferral block state per context (#30); rename legacy_entries (#34 item 3) - #40

Merged
trissim merged 1 commit into
mainfrom
fix/30-34-atomic-block-context
Oct 12, 2026
Merged

trissim merged 1 commit into
mainfrom
fix/30-34-atomic-block-context

Conversation

@trissim

@trissim trissim commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #30. Addresses item 3 of #34 (items 1–2 are in context_manager.py, owned by another PR, so this does not close #34).

#30: block state belongs to the context that opened the block

Decision: what is per context and what is process-wide

Answer Owner Why
Which atomic / atomic_success block am I inside (label, triggering scope, snapshot requested, entry signature, nesting depth) per context (ContextVar) It answers "whose block is this", the same question config_context answers for its scope.
How deep am I inside defer_live_invalidations per context (ContextVar) Same question for the deferral block.
Registered states, snapshot history, timelines, limbo, graveyard, callbacks process-wide (class attributes) These are what the blocks act on; there is one registry per process. Now stated in the class docstring, as the issue asks.
The queue of deferred invalidations (_deferred_invalidations) process-wide It is a debt owed to the shared states' caches. It is flushed later, outside the block that queued it (pyqt-reactive flushes from a separate call; record_snapshot flushes before any snapshot), so it cannot belong to the block.

Before, one class attribute was the nesting counter for every thread, so "depth 2" could mean one thread nested twice or two threads once each, and nothing retained could tell them apart ([P10] Proposition 6.2). The same location was also one thread's own block state and another thread's reading of "am I in a block" ([P9] Proposition 7.1). The two entry/exit bodies in atomic and atomic_success were identical, so they now share _enter_atomic_block / _leave_atomic_block while the two context managers stay distinct because their exit rules differ ([P1] Theorem 4.8). The atomic block and the deferral nesting give different answers and stay separate variables.

Mechanism

Searched src/objectstate for ContextVar and threading.local. context_manager.py keeps config_context scope in module-level contextvars.ContextVars; global_config.py uses threading.local for a different concern. The block state uses the first, the same mechanism as the context it nests in. No new kind of thread-local was added.

  • AtomicBlock (dataclass) holds one open block. _atomic_block: ContextVar[AtomicBlock | None] says which block the running context is in.
  • ObjectStateRegistry.current_atomic_block() is the one public read: the block, or None outside any block.
  • _live_invalidation_deferrals: ContextVar[int] replaces _deferred_invalidation_depth.
  • The five _atomic_* class attributes and _deferred_invalidation_depth are deleted.

Acceptance

The reproducer prints depth 0 with no label on the first line and depth 1 with label 'main edit' on the second.

Met. The reproducer is tests/test_atomic_block_context.py::test_thread_outside_any_block_does_not_see_another_threads_block: with a worker thread held inside atomic("worker edit"), the main thread reads (0, None) outside its block and (1, "main edit") inside. The reproducer read the private class attributes, which no longer exist; the test reads the same two facts through current_atomic_block().

The other tests in that file cover the consequences named in the issue: a snapshot requested by the main thread is recorded at once rather than absorbed by the worker's block; the main thread's own atomic / atomic_success records on exit; nesting in one context still shares the outer block; an asyncio task started inside a block inherits it and a sibling task does not (same rule as config_context); a deferral open in one thread does not queue another thread's invalidation. All six failed on origin/main before the fix.

#34 item 3: TimeTravelChangeSet.legacy_entries

Case: it is the current format, so it was renamed, not replaced.

Evidence: TimeTravelCompleteCallback is Callable[[List[Tuple[str, ObjectState]], Optional[str]], None] and is the only completion-callback type; _fire_time_travel_complete_callbacks is the only producer and this method builds its payload. Every subscriber takes that shape as (dirty_states, triggering_scope): pyqt-reactive time_travel_navigation.py:123, abstract_manager_widget.py:165, function_list_editor.py:234; OpenHCS pipeline_editor.py:586, plate_manager.py:329, dual_editor_window.py. No callback takes a TimeTravelChangeSet.

New name: states_needing_navigation. Callers updated: object_state_registry.py (one) and tests/test_undo_redo_cascade.py (one).

External callers of legacy_entries: none. grep -rn "legacy_entries" over external/pyqt-reactive/src and openhcs/ returns nothing, so no matching change is needed at release time.

grep -n "legacy\|DEPRECATED" src/objectstate/object_state_registry.py now returns nothing.

Behaviour changes for callers (pyqt-reactive, OpenHCS)

  • A thread, or an asyncio task not started inside the block, no longer joins another context's atomic block or deferral. Code that opened a block on one thread and relied on a different thread's snapshots or invalidations being coalesced into it will now see those recorded/applied immediately. Single-threaded use (the Qt main thread) is unchanged.
  • ObjectStateRegistry._atomic_depth, _atomic_label, _atomic_triggering_scope, _atomic_snapshot_requested, _atomic_entry_signature and _deferred_invalidation_depth are gone. Neither pyqt-reactive nor OpenHCS reads them (grepped). Use current_atomic_block().
  • ObjectStateRegistry.clear() resets the block state of the calling context only. A block dropped by clear() while open does not record on exit (before, the depth went negative and it also did not record).
  • TimeTravelChangeSet.legacy_entries is renamed states_needing_navigation.
  • Not changed: the process-wide stores are still unsynchronized. Two threads that mutate the registry at the same time must still serialize those calls; the class docstring now says so.

Files outside object_state_registry.py

Tests only: the new tests/test_atomic_block_context.py; the rename in tests/test_undo_redo_cascade.py; and removal of the reset lines that assigned the deleted _atomic_* attributes in four test files' reset helpers (they would otherwise create dead attributes).

Checks

  • pytest -n 2: 246 passed.
  • ruff check src/ --select E9,F63,F7,F82: clean.
  • scripts/check_ruff_added_lines.py --base origin/main: no diagnostics on added lines.
  • No version bump.

🤖 Generated with Claude Code

… payload

atomic, atomic_success and defer_live_invalidations kept their nesting in
class attributes shared by the process, while config_context keeps its scope
in context variables. The open block is now an AtomicBlock held in a
ContextVar and the deferral nesting is a ContextVar count, so a thread
outside any block no longer sees another thread's block (#30).

TimeTravelChangeSet.legacy_entries is the only completion-callback payload,
so it is renamed states_needing_navigation (#34 item 3).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 12, 2026 04:26

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@trissim

trissim commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #50 (release/3.0.0-context-resolution), which merges this branch unchanged together with #41, #49, #40 and #42 for the 3.0.0 release. Left open; review and merge decisions happen on #50.

@trissim
trissim merged commit 598cb09 into main Oct 12, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants