Skip to content

Add custom coderabbit instructions#4

Merged
guill merged 2 commits intomainfrom
js/add-coderabbit
Jan 20, 2026
Merged

Add custom coderabbit instructions#4
guill merged 2 commits intomainfrom
js/add-coderabbit

Conversation

@guill
Copy link
Member

@guill guill commented Jan 19, 2026

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

Adds a .coderabbit.yaml configuration prescribing path-based governance for Python code, tests, examples, and benchmarks; enables sequence diagram and changed-files summaries for reviews. No runtime or public API changes.

Changes

Cohort / File(s) Summary
Configuration
.coderabbit.yaml
New repository governance config that: enables sequence diagrams and changed-files summaries; defines path-based rules for Python core (structure, zero runtime deps, async RPC, type hints, error handling, backwards compatibility), tests (pytest fixtures, slow integration handling, cautious mocking), examples (clarity, working and documented), and benchmarks (reproducibility, warm-ups, stats, GPU OOM guidance).

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @.coderabbit.yaml:
- Line 30: The guideline currently references TypeScript syntax ("@ts-ignore")
alongside Python's "# type: ignore"; update the rule to be Python-specific by
removing or replacing the TypeScript token "@ts-ignore" and any TypeScript
casting advice (e.g., "cast to Any") with Python equivalents (e.g., avoid "cast
to Any" or only use "# type: ignore" with justification). Locate the line
containing "# type: ignore", "@ts-ignore", and "Any" and ensure the text only
mentions Python constructs and examples, keeping the prohibition scoped to
Python type-ignores and unjustified use of typing.Any.
- Line 82: Remove the trailing blank line at the end of .coderabbit.yaml so the
file ends on the last YAML content line (no extra newline or empty line after
it); open the file, delete the final empty line character(s) after the last YAML
entry and save so YAML lint passes.

@guill guill merged commit ad64e40 into main Jan 20, 2026
35 of 40 checks passed
@guill guill deleted the js/add-coderabbit branch January 20, 2026 03:39
Kosinkadink pushed a commit that referenced this pull request Feb 14, 2026
… decoupling (comfy decoupling part 1)

## PR #3 Review Comments Addressed (38/38)

### Hardcoded Paths Removed (#2, #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, #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 (#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
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.

1 participant