Skip to content

refactor(cql): remove vestigial load_graph fetch stack from loader.py (BE-4364)#598

Open
mattmillerai wants to merge 1 commit into
mainfrom
matt/be-4364-remove-loader-fetch-stack
Open

refactor(cql): remove vestigial load_graph fetch stack from loader.py (BE-4364)#598
mattmillerai wants to merge 1 commit into
mainfrom
matt/be-4364-remove-loader-fetch-stack

Conversation

@mattmillerai

Copy link
Copy Markdown
Collaborator

ELI-5

comfy_cli/cql/loader.py had 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 loads object_info through resilient_load_object_info, which goes to the engine's loaders. This PR deletes that dead code and its tests, and makes resilient_load_object_info the public entry point instead of load_graph. No behavior changes.

What changed

  • comfy_cli/cql/loader.py: deleted load_graph, the loader's own _load_from_file, and _load_from_server (the whole block above the # ---- normalization ---- divider), plus the now-orphaned _LOADER_OPENER and MAX_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-export resilient_load_object_info as the public entry point in place of load_graph.
  • tests/comfy_cli/cql/test_loader.py: removed the tests covering the deleted stack (test_load_graph_*, test_load_from_server_*, _FakeResp); kept all normalize tests and fixtures.

Why it's safe (security-flagged removal)

The engine's loaders (comfy_cli/cql/engine.py _load_from_file/_load_from_target) and resilient_load_object_info are 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 in loader.py, eliminating the security-policy fork (256 MiB vs 64 MiB byte caps, duplicate SSRF guards) flagged in BE-4352. normalize and 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_info is 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 --check clean on the touched files (the pre-existing default_workflow.py:139 UP038 lint is unrelated and left untouched).

…_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.
@mattmillerai mattmillerai added agent-coded PR authored by the agent-work loop cursor-review Request Cursor bot review labels Jul 24, 2026
@mattmillerai
mattmillerai marked this pull request as ready for review July 24, 2026 09:57
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 62804700-6376-4f88-8204-2da341463b0b

📥 Commits

Reviewing files that changed from the base of the PR and between 85b62da and 68be926.

📒 Files selected for processing (3)
  • comfy_cli/cql/__init__.py
  • comfy_cli/cql/loader.py
  • tests/comfy_cli/cql/test_loader.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch matt/be-4364-remove-loader-fetch-stack
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch matt/be-4364-remove-loader-fetch-stack

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

@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 24, 2026

@github-actions github-actions Bot 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.

🔍 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent-coded PR authored by the agent-work loop cursor-review Request Cursor bot review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant