Conversation
pollockjj
referenced
this pull request
in pollockjj/pyisolate
Dec 16, 2025
… decoupling (comfy decoupling part 1) ## PR Comfy-Org#3 Review Comments Addressed (38/38) ### Hardcoded Paths Removed (Comfy-Org#2, Comfy-Org#3, #13, #14, #36) - Remove /home/johnj references from tests and documentation - Use COMFYUI_ROOT env var with $HOME/ComfyUI fallback in conftest.py - Delete SETUP_GUIDE.md; consolidate in README.md - Add comfy_hello_world/node-venvs/ to .gitignore ### Error Messages & Documentation (#1, Comfy-Org#5, #7, #8) - Improve site-packages error with host_prefix, valid_parents, candidates - Restore editable install comment explaining -e flag behavior - Add 50-line architecture docstring to shared.py documenting three-layer serialization (Layer 1: _prepare_for_rpc, Layer 2: serialize_for_isolation, Layer 3: RPC envelope) - Clarify zero-copy: CUDA IPC on Linux keeps tensors on-device; otherwise CPU shared memory fallback ### Fail-Loud Pattern (Comfy-Org#4, #21, #22, #23, #24, #25, #26, #27, #29, #33, #35) - Restore try/except in client.py with fail-loud logging - Replace hasattr duck-typing with exact type_name matching for CLIP/VAE - Delete unused move_tensors_to_device function (VRAM leak risk) - Remove ALL truncation/defensive code; raise RuntimeError when objects lack _instance_id instead of silently returning opaque repr - Remove module allowlists entirely (ALLOWED_MODULES patterns eliminated) - _tensor_to_cuda now passthrough; no auto-migration of tensors to CUDA ### Code Clarity (#18, #19, #20, #28, #30, #31, #32, #34) - Remove "ComfyUI" hardcoded assumption; derive preferred_root from snapshot - Remove original_register_route replacement logic (simplified) - Add 5-line comment explaining .replace(".", "_x_") for Python identifiers - Restore singleton bypass comment (object.__new__ to prevent recursion) - Confirm handling_call_id IS read for parent_call_id tracking - Rename _tensor_to_cpu → _prepare_for_rpc (12 occurrences) - Add RPC message protocol comments (RPCRequest/RPCCallback/RPCResponse) - Add else clause raising ValueError for unknown RPC request kinds ### Code Quality (#15, #16, #17, #37, #38) - Move get_torch_ecosystem_packages() to _internal/torch_utils.py - Propagate only host env vars (no defaults added) - already correct - Ensure ruff/mypy pass with zero errors - Compare BOTH fingerprint AND full descriptor before skipping install ## New Features ### Adapter Decoupling Architecture - NEW: pyisolate/interfaces.py - IsolationAdapter protocol + SerializerRegistryProtocol - NEW: pyisolate/_internal/bootstrap.py - Child process bootstrap resolving "config before path" paradox - NEW: pyisolate/_internal/loader.py - Entry point discovery for adapters (pyisolate.adapters group) - NEW: pyisolate/_internal/serialization_registry.py - Dynamic serializer registry with O(1) lookup - host.py: build_extension_snapshot() integrates adapter metadata - client.py: bootstrap_child() applies snapshot before heavy imports ### Test Coverage ~40% → 96% - NEW: 23 test files added - tests/test_bootstrap.py, test_bootstrap_additional.py - tests/test_loader.py, test_loader_additional.py - tests/test_serialization_registry.py - tests/test_model_serialization.py, test_model_serialization_additional.py - tests/test_shared_unit.py, test_shared_additional.py, test_shared_rpc_flows.py - tests/test_host_unit.py, test_host_additional.py, test_host_public.py, test_host_internal_ext.py, test_host_integration.py - tests/test_client_entrypoint_extra.py - tests/test_remaining_coverage.py (comprehensive edge cases) - tests/test_fail_loud.py - Reorganize integration tests to tests/integration/ ### Type Annotations - Add type hints to AttrDict, AttributeContainer, AsyncRPC - Add __all__ export list to host.py - Use dict[str, Any] instead of Dict for modern Python
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.