refactor(cql): remove vestigial load_graph fetch stack from loader.py (BE-4364)#598
refactor(cql): remove vestigial load_graph fetch stack from loader.py (BE-4364)#598mattmillerai wants to merge 1 commit into
Conversation
…_server stack (BE-4364) The loader's private fetch stack (load_graph, its own _load_from_file, and _load_from_server) is dead: every production caller (command/nodes.py, command/workflow.py, command/workflow_fragments.py) reaches object_info via resilient_load_object_info, which delegates to the engine's loaders. The loader's stack was reached only by its own tests and the cql/__init__.py export. Delete that block (through the # ---- normalization ---- divider), the now-orphaned _LOADER_OPENER and MAX_INPUT_BYTES, and the imports they used (NoRedirectHandler, is_loopback_host, urllib.*). Rewrite the module docstring to describe what remains: normalize and resilient_load_object_info. Re-export resilient_load_object_info as the public cql entry point in place of load_graph. Drop the tests that covered the removed stack; keep all normalize tests. The engine's live loaders and resilient_load_object_info are untouched — they keep their own loopback guard, no-redirect opener, byte cap, and cloud HTTPS+auth. This removes the security-policy fork (256 MiB vs 64 MiB byte caps, duplicate SSRF guards) flagged in BE-4352.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
✅ No high-signal findings.
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
ELI-5
comfy_cli/cql/loader.pyhad an old, private way to fetch a workflow graph (load_graph→_load_from_file/_load_from_server). Nothing in the product uses it anymore — every real command loadsobject_infothroughresilient_load_object_info, which goes to the engine's loaders. This PR deletes that dead code and its tests, and makesresilient_load_object_infothe public entry point instead ofload_graph. No behavior changes.What changed
comfy_cli/cql/loader.py: deletedload_graph, the loader's own_load_from_file, and_load_from_server(the whole block above the# ---- normalization ----divider), plus the now-orphaned_LOADER_OPENERandMAX_INPUT_BYTES. Pruned the imports they used (NoRedirectHandler,is_loopback_host,urllib.error/parse/request). Rewrote the module docstring to describe what remains (normalize+resilient_load_object_info).comfy_cli/cql/__init__.py: re-exportresilient_load_object_infoas the public entry point in place ofload_graph.tests/comfy_cli/cql/test_loader.py: removed the tests covering the deleted stack (test_load_graph_*,test_load_from_server_*,_FakeResp); kept allnormalizetests and fixtures.Why it's safe (security-flagged removal)
The engine's loaders (
comfy_cli/cql/engine.py_load_from_file/_load_from_target) andresilient_load_object_infoare the live path and are untouched — they keep their own loopback guard, no-redirect opener, byte cap, and cloud HTTPS+auth. This change only removes the vestigial duplicate stack inloader.py, eliminating the security-policy fork (256 MiB vs 64 MiB byte caps, duplicate SSRF guards) flagged in BE-4352.normalizeand its tests are intentionally left in place (a separate follow-up).Public API note (for release notes)
comfy_cli.cql.load_graph— which was exported via__all__— is removed.resilient_load_object_infois the replacement entry point. Flagging so reviewers can note it in release notes.Verification
grep -rn "load_graph\|_load_from_server\|_LOADER_OPENER\|MAX_INPUT_BYTES" --include='*.py' .→ zero hits (excluding.egg-info).uv run pytest tests/comfy_cli/cql/ tests/comfy_cli/command/→ 1409 passed, 2 skipped.ruff check/ruff format --checkclean on the touched files (the pre-existingdefault_workflow.py:139UP038 lint is unrelated and left untouched).