fix(cql): answer same-type nodes path queries instead of declining them

[mattmillerai]

Base is now main. This PR was opened stacked on #695 (matt/be-6857-nodes-path-type-constraint); #695 has merged and GitHub retargeted this to the default branch. The earlier STACKED banner no longer applies.

ELI-5

Ask the CLI "how do I get from a MODEL to a MODEL?" and it used to shrug: zero routes, plus a note saying it hadn't actually looked. But there is an obvious answer — a LoRA loader takes a MODEL and hands you back a MODEL. It does that all day. The tool just refused to say so.

Two things were in the way. A guard at the top bailed out whenever the start and end types matched, before any searching happened. And the search itself had a rule that throws away any step handing back the type it was given — a sensible rule for a step in the middle of a route (that step accomplishes nothing), but for a same-type question that is the only step that could answer it.

Now the guard is gone and the rule makes an exception for the final step. comfy nodes path MODEL MODEL lists LoraLoaderModelOnly. Every other question you can ask gets exactly the same answer it did before.

What changed

• comfy_cli/cql/engine.py::Graph.search_paths — removed the early from_type == to_type abstention; changed the no-op rule from if out_t == cur_type to if out_t == cur_type and out_t != to_type. Docstring updated: degenerate_bounds is now the sole not_searched_reason.
• comfy_cli/command/nodes.py::path_cmd — docstring no longer claims same_type is the reason reachable from the CLI. No not_searched reason is reachable from the CLI at all now: the only shape search_paths declines is a bound below 1, which path_cmd already rejects up front with path_bounds_invalid.
• Tests updated per the plan, plus one new guard test (below).

Why the exemption cannot leak into other queries

A step whose output matches to_type is appended as a completed path and continued — it is never pushed onto the queue. So cur_type == to_type can only hold for the initial frontier item, which means only when from_type == to_type. Every other search is untouched.

Verified rather than argued: a differential harness ran search_paths on this branch and on the base commit over 1568 combinations — every FROM/TO pair of every type in both catalog fixtures (nodes_path_object_info.json, sd15_object_info.json) × exact/loose × max_depth ∈ {1,2,3,6} × max_paths ∈ {1,10}, excluding the same-type pairs that are the intended change. The serialized results are byte-identical.

Empirical check through the real CLI

$ comfy --json nodes path MODEL MODEL --input tests/comfy_cli/fixtures/nodes_path_object_info.json
{"count": 1, "not_searched": false, "not_searched_reason": null,
 "paths": [{"from": "MODEL", "to": "MODEL",
            "steps": [{"node": "LoraLoaderModelOnly", "from_type": "MODEL", "to_type": "MODEL"}],
            "support": []}], ...}

Against the sd15 fixture, LATENT -> LATENT returns KSampler with exact: true — a same-type answer that carries the full exhaustiveness claim.

Judgment calls and one unmet acceptance criterion

1. exact: true on MODEL MODEL is NOT met, deliberately. The route is listed and not_searched is false, but the envelope reports exact: false because collapsed is true. That collapse is genuine and unrelated to this change: reaching MODEL ends a path, so the walk keeps expanding the branches that do not end (KSampler -> LATENT), and there both VAEDecode and VAEDecodeTiled land on the same (IMAGE, {IMAGE, LATENT}) state. #695 deliberately made collapsed err toward true, and forcing exact: true here would mean suppressing that flag — i.e. forging exactly the proof #695 was written to stop forging. So the test asserts collapsed is True / exact is False with the reason spelled out. The criterion holds wherever no unrelated collapse occurs, e.g. LATENT -> LATENT on the sd15 fixture above. Flagging it rather than hiding it: if reviewers want exact: true for same-type queries, that is a change to collapsed's reachability semantics and belongs in its own ticket.

2. The plan's fixture assumption was wrong for one test. It said to convert test_find_paths_same_type_returns_empty to assert graph.find_paths("MODEL","MODEL") returns the LoraLoaderModelOnly route — but that test uses the small inline graph fixture, which has no LoraLoaderModelOnly and no MODEL-consuming-MODEL-producing node at all. Only graph_path carries one. So that test now asserts the honest outcome for its own catalog (still empty, but not_searched is False — a fact about the catalog, not an abstention), and the positive route assertion lives in test_same_type_query_finds_the_route, which uses graph_path, exactly as the plan's item 3.4 specified.

3. Added one test beyond the plan. Removing an existing guard risks over-broadening, so test_no_op_hops_are_still_dropped pins the other half: for a FROM != TO query no step may hand back the type it consumed, and within a same-type query only the terminal hop may. Mutation-checked both ways — dropping the no-op rule entirely fails it, and restoring the old unconditional rule fails the capability tests.

Verification

• ruff check . and ruff format --check . — clean.
• Full suite (pytest) — 4374 passed, 36 skipped in 21m.
• pytest tests/comfy_cli/cql/test_engine.py tests/comfy_cli/command/test_nodes_introspect.py — 194 passed (re-run after the final test-only edits, which landed after the full-suite run started; all product code was final before it).
• Differential equivalence harness — 1568/1568 identical (above).

Note on capability direction

The falsification discipline applies inverted here: this diff removes a capability denial rather than adding one. The route it now surfaces was confirmed live through the CLI before the denial was deleted, not merely asserted by a test.

Post-review fix (2026-08-11)

@bigcat88 caught that the origin/main merge into this branch (86ed079) resolved nodes._get_graph to the
outgoing side, silently reverting #687 (BE-6660): the report_usage_error(get_renderer()) wrapper around
resolve_host_port was deleted. That made every comfy nodes verb exit 2 with zero bytes on stdout for a
rejected --host/--port instead of emitting the terminating host_port_invalid envelope — the exact
stream-just-stops failure BE-6660 was filed for, and the red build check.

Reproduced locally on the merged tree before fixing:
test_host_port.py::test_nodes_bad_port_terminates_with_envelope failed deterministically
(-p no:randomly: 1 failed, 79 passed), and passes with the wrapper restored. Restored verbatim from main;
the only remaining nodes.py delta versus main is this PR's path_cmd docstring. Current origin/main
merged in at the same time.

[coderabbitai]

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: 12 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: 6131d730-b488-45da-8216-e2d6c9ab1728

📥 Commits

Reviewing files that changed from the base of the PR and between 8d7ad2b and 3b0bb2d.

📒 Files selected for processing (4)

• comfy_cli/command/nodes.py
• comfy_cli/cql/engine.py
• tests/comfy_cli/command/test_nodes_introspect.py
• tests/comfy_cli/cql/test_engine.py

---

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

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

⚠️ Panel did not produce any findings.

Every reviewer in the matrix failed to contribute — see the panel summary for which cells errored, and the run logs for the underlying cause.

Panel: 0/8 reviewers contributed findings.

Reviewers that did not contribute: claude-opus-5-thinking-max:adversarial (error), gemini-3.1-pro:adversarial (error), gpt-5.6-sol-max:adversarial (error), kimi-k3-max:adversarial (error), claude-opus-5-thinking-max:edge-case (error), gemini-3.1-pro:edge-case (error), gpt-5.6-sol-max:edge-case (error), kimi-k3-max:edge-case (error)

[mattmillerai]

Independent re-verification of this branch (fresh worktree at 4b2e20f, deps via uv sync --frozen --extra dev), re-running the claims in the description rather than taking them on trust:

• Acceptance tests — pytest tests/comfy_cli/cql/test_engine.py tests/comfy_cli/command/test_nodes_introspect.py → 194 passed in 21s.
• Non-same-type equivalence, re-derived independently — a separate harness (not the one in the description) enumerated every FROM/TO pair over both catalog fixtures × exact/loose × max_depth ∈ {1,2,3,6} × max_paths ∈ {1,10}, excluding same-type pairs, and compared serialized search_paths output against the base commit d66bc22: 1152/1152 byte-identical.
• The scoping argument, checked against the code rather than the comment — every state whose out_t == to_type hits paths.append(...) + continue and is never appended to next_queue; the queue is seeded only with from_type. So cur_type == to_type can hold only for the seed item, i.e. only when from_type == to_type. The new clause therefore differs from the old one strictly when out_t == cur_type == to_type, which is exactly the same-type case.
• CLI acceptance criterion — comfy --json nodes path MODEL MODEL --input tests/comfy_cli/fixtures/nodes_path_object_info.json returns count: 1, the one-step LoraLoaderModelOnly route, not_searched: false. exact is false (collapsed: true) — the deliberate deviation already documented under "Judgment calls" above.
• The one remaining empty assertion, falsified rather than assumed — test_find_paths_same_type_is_searched_not_declined asserts find_paths("MODEL","MODEL") == [] on the small inline fixture. Checked directly: only CheckpointLoaderSimple emits MODEL in that catalog and it consumes none, so no MODEL-in/MODEL-out node exists. The empty result is a property of the fixture, not a restatement of the test's own premise.

One pre-existing lint note, not from this diff: ruff check flags UP038 at comfy_cli/cql/engine.py:1463 (isinstance(spec, (list, tuple))), well outside the changed region. It reproduces identically on the base branch d66bc22, so it is ruff-version drift against the pinned CI version, not a regression here — repo CI is green.

[bigcat88]

Needs a rebase onto main before I can review it.

Its base #695 landed as squash merge 5ebc831, so this branch's own copies of those commits no longer match what's on main — merging origin/main in now conflicts:

CONFLICT (content): Merge conflict in tests/comfy_cli/command/test_nodes_introspect.py
CONFLICT (content): Merge conflict in tests/comfy_cli/cql/test_engine.py

Both are test files where #695's cases and this PR's same-type cases were added in the same regions. The single commit that is genuinely unique here is 4b2e20f fix(cql): answer same-type path queries instead of declining them, so a git rebase --onto origin/main pr695 (or cherry-picking that one commit onto a fresh branch off main) should reduce this to a small diff.

Context from reviewing #695: I confirmed the abstention this PR removes is live on main right now —

comfy --json nodes path IMAGE IMAGE --input <real object_info>
  -> count=0  exact=false  not_searched=true  not_searched_reason="same_type"

so the behaviour you're fixing is real and reachable. Worth keeping in mind for the rebased version: on a real 792-class catalog, hub types like IMAGE collapse or truncate almost immediately, so a same-type answer will realistically come back exact: false — make sure the new tests don't assume exact: true on anything but a small fixture.

Ping me once it's rebased and I'll pick it up.

[mattmillerai]

@bigcat88 merged `origin/main` in (`86ed079`), resolving the conflicts in `comfy_cli/command/nodes.py`, `comfy_cli/cql/engine.py`, and both test files — kept both #695's source-type-constraint cases and this PR's same-type cases. The PR diff is now scoped back down to the 4 files / single logical change (`4b2e20f`); pushed and CI is re-running. Ready for your review whenever you get a chance.

[bigcat88]

The same-type fix itself is correct and I verified it independently — but the branch also silently reverts #687, which is what your red build check is. One deletion to undo and this is good to go.

The blocker: report_usage_error is gone from nodes._get_graph

git diff origin/main... shows this branch removing the wrapper that #687 (BE-6660, merged yesterday) added:

-        with report_usage_error(get_renderer()):
-            host, port = resolve_host_port(host, port)
+        host, port = resolve_host_port(host, port)

Nothing in the PR body mentions it, and it has nothing to do with same-type queries — it reads like a merge resolution that dropped the incoming side. git log -S confirms the wrapper's only author is #687.

Measured through the real CLI, after merging current origin/main into this branch:

comfy --json … --port 0	main	this branch
nodes ls	rc=2, 396 B host_port_invalid envelope	rc=2, 0 B on stdout
nodes show KSampler	rc=2, 396 B envelope	rc=2, 0 B
nodes path MODEL IMAGE	rc=2, 396 B envelope	rc=2, 0 B

Also 0 bytes for the --host 1.2.3.4:0 form and in NDJSON mode. That is exactly the "machine consumer just sees the stream stop" failure BE-6660 was filed for, restored for every comfy nodes verb.

tests/comfy_cli/test_host_port.py::test_nodes_bad_port_terminates_with_envelope fails deterministically on the merged branch (-p no:randomly: 1 failed, 79 passed) — it is not the flake the single red check might suggest.

Fix: restore the two lines (and the report_usage_error import) in _get_graph. Nothing else in the diff needs to change.

What I verified about the actual change (all good)

The no-op exemption does not leak — independently reproduced, not taken on trust. I ran search_paths over every FROM/TO type pair × exact/loose × max_depth ∈ {1,2,3} on both trees:

catalog	cross-type queries	changed
live server, 792 classes, 109 link types	70,632	0
nodes_path_object_info.json	252	0
sd15_object_info.json	180	0
subgraph_object_info.json	252	0

All 654 same-type queries on the real catalog changed, exactly as intended — 225 of them from an abstention to a list of real routes. Your reasoning for why cur_type == to_type can only hold on the initial frontier item holds up empirically at this scale.

Live CLI, against a real ComfyUI on :8189:

main:   nodes path MODEL MODEL -> count=0  not_searched=true  reason=same_type
branch: nodes path MODEL MODEL -> count=10 not_searched=false  (APG, CFGNorm, CFGOverride, …)
        nodes path LATENT LATENT -> count=3 (AddNoise, HunyuanRefinerLatent, …)

Real MODEL-patching nodes, which is the answer a user asking that question actually wants.

I also agree with the call in your "unmet acceptance criterion" note: reporting exact: false because collapsed is genuinely true is the right outcome. Forcing exact: true there would forge exactly the proof #695 exists to prevent. Leave it.

Two minor things while you're in here

• The branch is 5 commits behind main (#694, #690, #528, #641, #699 landed today). Worth a git merge origin/main with the fix.
• The body still carries the STACKED — merging lands on matt/be-6857… banner; #695 has merged and GitHub retargeted this to main.

[mattmillerai]

Confirmed and fixed in 3b0bb2d — you were right on every detail, including the cause.

git show 86ed079 shows the merge resolving _get_graph to the outgoing side: parent 4b2e20f (this branch, pre-merge) has the bare resolve_host_port call, parent 87f0414 (main) has your wrapper, and the merge kept the branch's. Nothing about it was intentional — the same-type change never touches _get_graph.

Reproduced before fixing, on the tree with current origin/main merged in:

$ pytest tests/comfy_cli/test_host_port.py -p no:randomly -q
FAILED test_host_port.py::test_nodes_bad_port_terminates_with_envelope
  AssertionError: stdout was empty; stderr: Usage: root nodes ls [OPTIONS] ...
1 failed, 79 passed

Restored the two lines verbatim from main (wrapper, comment, and the report_usage_error import) — same suite is now 80 passed, and the full suite is 4650 passed / 36 skipped.

The only remaining nodes.py delta versus main is this PR's path_cmd docstring, so nothing else from #687 is at risk:

$ git diff origin/main..HEAD -- comfy_cli/command/nodes.py --stat
 comfy_cli/command/nodes.py | 14 +++++++-------

Both minor points handled too: current origin/main is merged in (5073111), and the PR body's stale STACKED banner is replaced with a note that #695 merged and GitHub retargeted this to main.

Thanks for the differential run over 70,632 cross-type queries — that is a much stronger check on the exemption than my 1,568-combination fixture harness, and against a real 792-class catalog rather than fixtures.