Skip to content

R1: only serving readiness completes registries (fixes selected-source discovery regressed by #763) - #1217

Merged
trissim merged 7 commits into
mainfrom
refactor/R1-selected-source-discovery
Oct 12, 2026
Merged

trissim merged 7 commits into
mainfrom
refactor/R1-selected-source-discovery

Conversation

@trissim

@trissim trissim commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

What was wrong

tests/unit/agent/test_declaration_selected_discovery.py::test_examplehuman_selected_source_uses_same_declaration_owner fails on main. The test is not stale. 91f05c26b (#763) made CellProfilerModule a CompilerPreparedAutoRegisterFamily whose hook reads every registry member, and _scan_module_registries also counted the members of every prepared family. Pipeline import prepares callables, so retrieving one knowledge-base example imported all 91 CellProfiler declarations.

Change

  • CallablePreparation.prepare_for_serving (new, openhcs/core/processing_preparation.py) prepares every callable and then completes the lazy registries their modules use, once, through AutoRegisterRegistryPreparation.discover_module_registries (previously uncalled). RegistryService.prepare_in_current_process calls it. This keeps Complete CellProfiler measurement registry discovery before READY #763's requirement that registries are discovered before READY.
  • Preparing one callable now only runs family hooks; it never reads a registry's members.
  • Guard: tests/unit/test_callable_contract.py::test_only_serving_readiness_reads_the_members_of_a_registry.

Deletions

  • CellProfilerModule.prepare_registered_family and its CompilerPreparedAutoRegisterFamily base: derived (serving readiness now completes the registry).
  • AutoRegisterRegistryPreparationReport and its counts, _scan_module_registries: unused (only a test read prepared_family_count).

Verification (local, Linux, merged with origin/main ae6ddace5, head 6b9398566; the later commit changes the receipt only)

  • The fixed test file with test_callable_contract.py and test_library_registry_discovery.py: 62 passed in 91.34s.
  • Twelve preparation and registry neighbour files (test_processing_preparation, test_execution_server_startup_preparation, test_fork_compiler_preparation, test_preparation_admission, test_callable_abi_preparation, test_registry_owner_iteration, test_registry_library_boundary, test_colocalization_preparation_readiness, test_cellprofiler_kernel_preparation, test_cellprofiler_callable_kernel_preparation, test_cellprofiler_strategy_registries, test_func_registry_projection): 114 passed in 188.37s.
  • guardrails_local.sh: guardrails rc=0. SMOKE_SKIP_GUI=1 smoke_local.sh: 5 passed, smoke rc=0.
  • Official30 parity under flock: the full run did not pass as one run. It ended 1 failed with 27 of 30 cases equivalent (0 differences each). ExampleYeastPatches, cp_tutorial_advanced_segmentation_final and cp_tutorial_beginner_segmentation_final ended Execution was cancelled (two at compilation, one at execution), with no value comparison. Two other worktrees started execution servers on the same port (7849) 59 s and 72 s before the first two failures, and one of those servers logged a cancellation 5 s before the second. Those three cases, rerun alone under the lock through the test's own helper on a different port, were equivalent with 0 differences. So all 30 cases were observed equivalent, across two runs; a single clean 30/30 run was not observed.
  • Not checked: GPU libraries installed (readiness now completes about 60 lazy registries; CPU-only locally); macOS and Windows; the full unit suite.

Out of scope, found

The parity lock covers parity runs only. _free_zmq_port_pair in tests/integration/test_cellprofiler_official30_zmq.py picks the same first free port as other ZMQ tests on the machine, so an unlocked ZMQ test in another worktree can cancel a parity case.

Note: PR #1213 adds the same receipt file; whichever merges second needs a trivial merge of that document.

R1 (item 4) · PR #1217 · guards 1/1 · production −52 +47 · tests −17 +60

🤖 Generated with Claude Code

trissim and others added 6 commits October 11, 2026 22:52
…e does not

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RegistryService asks CallablePreparation.prepare_for_serving, which prepares
every callable and then completes the registries their modules use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iscovery

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

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.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@trissim
trissim merged commit 40b769c into main Oct 12, 2026
0 of 2 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

Development

Successfully merging this pull request may close these issues.

2 participants