diff --git a/docs/refactor/consolidation-20261009/R1-release-blockers.md b/docs/refactor/consolidation-20261009/R1-release-blockers.md index 110f07e07c..6aa9de5280 100644 --- a/docs/refactor/consolidation-20261009/R1-release-blockers.md +++ b/docs/refactor/consolidation-20261009/R1-release-blockers.md @@ -84,6 +84,7 @@ Each failure fixed at its cause, in its own PR. No global loosening of the parit **Done**: an autouse fixture freezes the pre-existing heap (`gc.freeze()` / `gc.unfreeze()`) so collections cover exactly what the test allocates; no assertion changed. Locally the file went from 21.4 s to 1.67 s (11 passed); one collection from 0.05–0.19 s to 30 µs. Mutation check: removing `self.destroyed.connect(subscription.cancel)` from `signals.py` still fails 5 tests under the fixture. A canary test shows a new cycle is still reclaimed. +**Remains**: guardrails, PR. **Findings for B6b:** approximate seconds per file in the cancelled run: `test_custom_function_events.py` 527, `agent/test_agent_services.py` 166 (one case also timed out at 120 s: `test_repository_catalog_finds_nucleus_segmentation_from_callable_owner`), `test_cellprofiler_kernel_preparation.py` 111, `test_configuration_mirror_deletion.py` 103, `test_cellprofiler_static_deletion_gates.py` 99, `agent/test_mcp_server.py` 94. A session heap that costs more than 3 s per full collection suggests tests leak tracked objects across the session. **Verified on `origin/main` `77bf7e803` merged in**: the file and `test_custom_function_lifecycle.py` pass (66 passed, 5.2 s); local guardrails end `rc=0`. PR opened; it remains to merge. **Findings for B6b:** approximate seconds per file in the cancelled run: `test_custom_function_events.py` 527, `agent/test_agent_services.py` 166 (one case also timed out at 120 s: `test_repository_catalog_finds_nucleus_segmentation_from_callable_owner`), `test_cellprofiler_kernel_preparation.py` 111, `test_configuration_mirror_deletion.py` 103, `test_cellprofiler_static_deletion_gates.py` 99, `agent/test_mcp_server.py` 94. A session heap that costs more than 3 s per full collection suggests tests leak tracked objects across the session. ### Item 4: unowned failing test — branch `refactor/R1-selected-source-discovery`, fix done, PR not opened @@ -92,6 +93,7 @@ Each failure fixed at its cause, in its own PR. No global loosening of the parit **Done**: `RegistryService.prepare_in_current_process` now completes the registries its callables' modules use (`AutoRegisterRegistryPreparation.discover_module_registries`, previously uncalled), which keeps #763's requirement "discovered before READY". Preparing one callable only runs family hooks. `CellProfilerModule.prepare_registered_family` and its `CompilerPreparedAutoRegisterFamily` base are deleted (**derived**: readiness discovery now owns it); `AutoRegisterRegistryPreparationReport` and its counts are deleted (**unused**: only a test read `prepared_family_count`). 158 tests passed across the eleven preparation and discovery test files, including the target test. +**Verified on merged main `ae6ddace5` (2026-10-12)**: guardrails rc=0, smoke rc=0. Parity under flock: 27 of 30 equivalent in the full run; `ExampleYeastPatches`, `cp_tutorial_advanced_segmentation_final` and `cp_tutorial_beginner_segmentation_final` ended "Execution was cancelled" while two other worktrees started execution servers on the same port (7849); those three, rerun alone under the lock on another port, were equivalent with 0 differences. **Finding (owner: the parity test or the dispatch rules):** the lock covers parity runs only, and `_free_zmq_port_pair` picks the same first free port as every other ZMQ test on the machine, so an unlocked ZMQ test in another worktree can cancel a parity case. **Remains**: The readiness step discovers every registry referenced by callable modules, about 60 lazy registries; locally each took under 0.01 s and no new top-level package was imported in CPU-only mode. Not checked with GPU libraries installed. **Remains**: full 30-case parity under flock (readiness preparation changed), guardrails, PR. The readiness step discovers every registry referenced by callable modules, about 60 lazy registries; locally each took under 0.01 s and no new top-level package was imported in CPU-only mode. Not checked with GPU libraries installed. ## Done when diff --git a/openhcs/core/autoregister_preparation.py b/openhcs/core/autoregister_preparation.py index 30bd69b3d1..04ac69ae67 100644 --- a/openhcs/core/autoregister_preparation.py +++ b/openhcs/core/autoregister_preparation.py @@ -3,7 +3,6 @@ from __future__ import annotations import sys -from dataclasses import dataclass from functools import lru_cache from types import ModuleType from typing import Iterable @@ -15,15 +14,6 @@ AUTO_REGISTER_REGISTRY_ATTRIBUTE = "__registry__" -@dataclass(slots=True) -class AutoRegisterRegistryPreparationReport: - """Summary of AutoRegister registries prepared before worker execution.""" - - registry_count: int = 0 - class_count: int = 0 - prepared_family_count: int = 0 - - class AutoRegisterRegistryPreparation: """Prepare AutoRegisterMeta registries at compiler/runtime boundaries.""" @@ -31,37 +21,32 @@ class AutoRegisterRegistryPreparation: def prepare_module_registered_families( cls, modules: Iterable[ModuleType | None], - ) -> AutoRegisterRegistryPreparationReport: - """Prepare compiler-owned AutoRegister families defined in modules.""" - return cls._scan_module_registries(modules, prepare_families=True) + ) -> tuple[type, ...]: + """Run the preparation hook of each compiler-prepared family a module uses. + + Preparing one callable must not complete a lazy registry: reading a + registry's members imports its whole discovery package. A family whose + hook needs every member asks its own registry. + """ + families = cls.module_registry_owners(modules, compiler_prepared_only=True) + for family in families: + family.prepare_registered_family() + return families @classmethod def discover_module_registries( cls, modules: Iterable[ModuleType | None], - ) -> AutoRegisterRegistryPreparationReport: - """Force registry discovery without invoking backend family warmups.""" - return cls._scan_module_registries(modules, prepare_families=False) + ) -> tuple[type, ...]: + """Complete every lazy registry the modules use, without family warmups. - @classmethod - def _scan_module_registries( - cls, - modules: Iterable[ModuleType | None], - *, - prepare_families: bool, - ) -> AutoRegisterRegistryPreparationReport: - """Scan module registries and optionally prepare module-owned families.""" - report = AutoRegisterRegistryPreparationReport() - for candidate in cls.module_registry_owners( - modules, compiler_prepared_only=prepare_families - ): - registry = candidate.__registry__ - report.class_count += len(tuple(registry.values())) - report.registry_count += 1 - if prepare_families: - candidate.prepare_registered_family() - report.prepared_family_count += 1 - return report + Serving readiness calls this once for the whole catalog so that no + registry is first discovered during compilation or execution. + """ + families = cls.module_registry_owners(modules, compiler_prepared_only=False) + for family in families: + tuple(family.__registry__.values()) + return families @classmethod def module_registry_owners( @@ -77,8 +62,6 @@ def module_registry_owners( if not isinstance(module, ModuleType): continue for candidate in cls.module_registry_families(module): - # Counting an unrelated lazy registry would discover its whole - # package even though this path has no preparation hook to run. if compiler_prepared_only and not issubclass( candidate, CompilerPreparedAutoRegisterFamily ): diff --git a/openhcs/core/processing_preparation.py b/openhcs/core/processing_preparation.py index 4ff7b08efb..6c093f9d2b 100644 --- a/openhcs/core/processing_preparation.py +++ b/openhcs/core/processing_preparation.py @@ -224,6 +224,32 @@ def prepare(self) -> None: for operation in self.operations(): operation.prepare() + @classmethod + def prepare_for_serving( + cls, + callables: Iterable[Callable[..., object]], + status_callback: Callable[[str], None], + ) -> None: + """Prepare every callable, then complete the registries their modules use. + + Only this step completes lazy registries, so none is first discovered + during compilation or execution. Preparing one callable does not. + """ + preparations = tuple(cls.from_callable(func) for func in callables) + for preparation in preparations: + preparation.prepare() + status_callback( + f"Prepared callable {preparation.projection.module_name}.{preparation.projection.name}" + ) + status_callback("Completing the registries prepared callables use") + AutoRegisterRegistryPreparation.discover_module_registries( + importlib.import_module(module_name) + for module_name in dict.fromkeys( + preparation.projection.module_name for preparation in preparations + ) + if module_name is not None + ) + def cache_sources(self) -> tuple[PreparationOperation, ...]: module_name = self.projection.module_name if module_name is None: diff --git a/openhcs/interop/cellprofiler/module_declarations.py b/openhcs/interop/cellprofiler/module_declarations.py index 908d856020..01da17f64f 100644 --- a/openhcs/interop/cellprofiler/module_declarations.py +++ b/openhcs/interop/cellprofiler/module_declarations.py @@ -25,7 +25,6 @@ from openhcs.core.callable_contract import ( CallableContract, CallableImportIdentity, - CompilerPreparedAutoRegisterFamily, FunctionStepExecutionScope, ) from openhcs.core.config import ( @@ -157,7 +156,6 @@ class CellProfilerModule( CellProfilerModuleSettings, TableMeasurementRecordRowsMixin, CellProfilerMeasurementFeatureOwner, - CompilerPreparedAutoRegisterFamily, ABC, metaclass=AutoRegisterMeta, registry_config=RegistryConfig( @@ -192,13 +190,6 @@ class CellProfilerModule( boundary. """ - @classmethod - def prepare_registered_family(cls) -> None: - """Discover measurement declarations before execution readiness.""" - # Selected callable imports do not complete this lazy registry. - # Keep discovery on its owner, including later plugin registration. - tuple(cls.__registry__.values()) - @staticmethod def number_step_invocation_blocks( invocation_blocks: tuple[tuple["ModuleBlock", ...], ...], diff --git a/openhcs/processing/backends/lib_registry/registry_service.py b/openhcs/processing/backends/lib_registry/registry_service.py index 07fa80fb8a..7ac68cb271 100644 --- a/openhcs/processing/backends/lib_registry/registry_service.py +++ b/openhcs/processing/backends/lib_registry/registry_service.py @@ -197,12 +197,7 @@ def prepare_in_current_process( max_workers=os.cpu_count() or 1, status_callback=emit_status, ) - for func in callables: - preparation = CallablePreparation.from_callable(func) - preparation.prepare() - emit_status( - f"Prepared callable {preparation.projection.module_name}.{preparation.projection.name}" - ) + CallablePreparation.prepare_for_serving(callables, emit_status) emit_status("Resolving prepared callable signatures") for func in callables: CallableProjection.from_callable(func).warm_canonical_signature() diff --git a/tests/unit/test_callable_contract.py b/tests/unit/test_callable_contract.py index 9fef19287d..272d2ab211 100644 --- a/tests/unit/test_callable_contract.py +++ b/tests/unit/test_callable_contract.py @@ -514,6 +514,51 @@ def process(image): assert calls == {"related": 1, "unrelated": 0} +def test_only_serving_readiness_reads_the_members_of_a_registry() -> None: + """Preparing a callable runs family hooks; it never completes a registry.""" + + class CountingRegistry(dict): + member_reads = 0 + + def values(self): + type(self).member_reads += 1 + return super().values() + + hook_calls = [] + + class LazyFamily(CompilerPreparedAutoRegisterFamily, metaclass=AutoRegisterMeta): + __registry_key__ = "registry_key" + __skip_if_no_key__ = True + registry_key = None + + @classmethod + def prepare_registered_family(cls) -> None: + hook_calls.append(cls) + + LazyFamily.__registry__ = CountingRegistry(LazyFamily.__registry__) + module = ModuleType("tests.unit._lazy_family_module") + module.LazyFamily = LazyFamily + sys.modules[module.__name__] = module + AutoRegisterRegistryPreparation.cached_module_registry_families.cache_clear() + try: + prepared = AutoRegisterRegistryPreparation.prepare_module_registered_families( + (module,) + ) + assert prepared == (LazyFamily,) + assert hook_calls == [LazyFamily] + assert CountingRegistry.member_reads == 0 + + discovered = AutoRegisterRegistryPreparation.discover_module_registries( + (module,) + ) + assert discovered == (LazyFamily,) + assert hook_calls == [LazyFamily] + assert CountingRegistry.member_reads == 1 + finally: + sys.modules.pop(module.__name__, None) + AutoRegisterRegistryPreparation.cached_module_registry_families.cache_clear() + + def test_prepare_processing_callable_caches_equivalent_bound_method_hooks() -> None: module_name = __name__ @@ -557,12 +602,12 @@ def test_module_registered_family_preparation_runs_compiler_prepared_family_hook _AUTOREGISTER_PREPARED_TEST_FAMILY_CALLS = 0 AutoRegisterRegistryPreparation.cached_module_registry_families.cache_clear() - report = AutoRegisterRegistryPreparation.prepare_module_registered_families( + prepared = AutoRegisterRegistryPreparation.prepare_module_registered_families( (sys.modules[__name__],) ) assert _AUTOREGISTER_PREPARED_TEST_FAMILY_CALLS == 1 - assert report.prepared_family_count == 1 + assert prepared == (_PreparedAutoRegisterFamily,) assert _PreparedAutoRegisterImplementation in ( _PreparedAutoRegisterFamily.__registry__.values() ) diff --git a/tests/unit/test_library_registry_discovery.py b/tests/unit/test_library_registry_discovery.py index 73fc7c4082..492a5ae1fb 100644 --- a/tests/unit/test_library_registry_discovery.py +++ b/tests/unit/test_library_registry_discovery.py @@ -765,10 +765,10 @@ def test_library_registry_discovery_is_stable_across_fresh_worker_processes( assert tuple((tmp_path / "cache" / "metaclass-registry").glob("*.json")) -def test_registered_metadata_preparation_discovers_measurements_before_lookup( +def test_serving_readiness_completes_the_measurement_registry_before_lookup( tmp_path: Path, ) -> None: - """Warm discovery once while later plugin declarations remain visible.""" + """Readiness discovers once; later plugin declarations remain visible.""" environment = os.environ.copy() environment.update( OPENHCS_CPU_ONLY="true", @@ -776,28 +776,26 @@ def test_registered_metadata_preparation_discovers_measurements_before_lookup( XDG_DATA_HOME=str(tmp_path / "data"), ) script = textwrap.dedent(""" - from openhcs.core.processing_preparation import CallablePreparation, RegistryFamilyPreparation + from types import SimpleNamespace + + from openhcs.core.processing_preparation import CallablePreparation, PreparationCacheBatch from openhcs.interop.cellprofiler.module_declarations import CellProfilerModule from openhcs.processing.backends.cellprofiler.thresholding import threshold + from openhcs.processing.backends.lib_registry.registry_service import RegistryService assert not CellProfilerModule.__registry__._discovered - operations = tuple( - operation - for source in CallablePreparation.from_callable(threshold).cache_sources() - for operation in source.cache_operations() - ) - preparation = next( - operation for operation in operations - if isinstance(operation, RegistryFamilyPreparation) - and operation.family is CellProfilerModule - ) - preparation.prepare() + # Kernel compilation is not this test's subject. + CallablePreparation.prepare = lambda self: None + PreparationCacheBatch.populate_child_caches = lambda *args, **kwargs: None + RegistryService._metadata_cache = {"threshold": SimpleNamespace(func=threshold)} + + RegistryService.prepare_in_current_process() assert CellProfilerModule.__registry__._discovered declarations = tuple(CellProfilerModule.__registry__.values()) assert declarations prefixes = CellProfilerModule.measurement_category_prefix_declarations() assert prefixes - preparation.prepare() + RegistryService.prepare_in_current_process() assert tuple(CellProfilerModule.__registry__.values()) == declarations class WarmupPlugin(CellProfilerModule):