feat(cli): nodes search --expand-top N - #707
Merged
skishore23 merged 1 commit intoAug 14, 2026
Merged
Conversation
Kill the measured search -> show xN agent loop (x76, ~92% of show args are a verbatim copy of the search hit): --expand-top N re-resolves the top-N returned hits through the exact catalog path nodes show uses (graph.node -> morphism_to_dict) and attaches each show payload under data.expanded[] with a class_type join key. - --expand-top 0 / omitted: payload byte-identical to today. - per-hit catalog miss degrades to a per-hit expand_miss error entry inside expanded[]; the search itself never fails. - expand_miss registered in the error-code registry (advisory, in-data). Linear: BE-7150 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
skishore23
added a commit
that referenced
this pull request
Aug 14, 2026
Brings four independent PR chains, all rooted on this branch, plus 32 commits of main drift. Chains (tip merges bring the whole chain): #704 op-vocab-freeze -> #715 bulk-op-emission -> #718 connect-LWW #705 --select -> #714 widget-catalog #706 --ack #707 expand-top -> #708 templates-get -> #709 path --emit-ops -> #710 delete-nodes All four merged with zero textual conflicts and zero test regressions. Op vocabulary doc is coherent at v1.2 (DEFERRED_OPS empty, reset_doc promoted in v1.1, concrete-input LWW in v1.2). origin/main conflicted in 8 files / 21 hunks. Five were not either/or -- both sides had added different things at the same spot, so keeping one side would have silently deleted the other's work: error_codes.py both new codes (expand_miss + path_bounds_invalid) nodes.py both dict keys (ops + support) loader.py main: kept `import time`, dropped urllib (dead after _net.py) registry/api.py both: 511's lazy `import requests` + main's timeout= DEFAULT_HTTP_TIMEOUT (main had already timed out the other three call sites in this file) preview.py main's CWD binary-planting guard + ffprobe/render timeouts, with 511's ffprobe-optional and imageio-ffmpeg fallbacks restored inside it. Both fallbacks fire only on a genuinely ABSENT binary; an untrusted PATH match is still refused, so the planting guard is intact. test_preview.py both test bodies cmdline.py main's deprecation of `comfy validate` into the shared validate_api_workflow, plus 511-only api_node_id (57:3 -> 57/3 subgraph remap) ported into that helper, which main lacked cql/engine.py hunk-by-hunk. None of 511's ten engine commits are in main, so taking main's file would have deleted the upload-backed enum fix, the MATCHTYPE wildcard, the reachability check and the SHA-256 subgraph fork id. Took main's dynamic-combo machinery (#574 / BE-3371, a superset) and its 6-tuple _parse_input_spec arity; restored Graph.widget_defaults and 511's structural {key,inputs} detection at the new arity. widgets_values guard: main's new code reads `node.get("widgets_values") or []` and then indexes positionally. A dict is truthy, so VHS_* nodes (which serialize widgets_values as a named dict) would raise KeyError: 0 again -- the defect behind 38 failures / 24% of in-scope in one 24h window, surfacing as "Could not extract slots: 0". Every read site now goes through _widgets_as_list, including two on main's NON-conflicting lines that the merge never flagged. Test tuning: two @patch targets in test_utils/test_standalone pointed at comfy_cli.utils.requests, which does not exist under the lazy import (verified: importing comfy_cli.utils does not load requests). Repointed to "requests.get" -- the pattern the same file already used elsewhere. Behaviour is unchanged; the timeout main added is still asserted. KNOWN FAILING (10) -- two clusters, both design decisions, not splices: 1. Dynamic-combo widget order (8). Two implementations of one feature. 511 expands sub-widgets in the static widget_order; main moved expansion into the value-aware _expand_widget_entries/_WidgetEntry. Tests from 511, main AND #714 each assert their own model, so no single side passes all three. Needs a call on which model wins. 2. run cloud lifecycle (2, plus test_run_prompt). The tests invoke a nonexistent wf.json expecting the mocked execute_cloud to short-circuit; a workflow_not_found check now fires first. Baselines measured, not assumed: 511 = 29 failed / 4603 passed; origin/main = 29 failed / 4617 passed. Lint is 19 errors, all UP038 style, zero F-class -- unchanged in kind from both baselines.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Linear: BE-7150 (V1-017) · Layer 1/4 of the V1 hop-killer stack (707 → 708 → 709 → 710), stacked on
fix/validate-lowers-ui-to-api.The hop it kills
Measured on prod agent traces:
nodes search→nodes show× N, ×76 occurrences, with ~92% of theshowargs a verbatim copy of the search hit. One flag folds the show payloads into the search envelope.Semantics
comfy nodes search <q> --expand-top Nre-resolves the top-N returned rows through the exact catalog pathnodes showuses (graph.node→morphism_to_dict) and attaches them underdata.expanded[], each entry{class_type, ...full show payload}(inputs withoptions.default/min/max, enumchoices,outputs,output_types, …) — the groundedshowfield vocabulary, not a new one.--expand-top 0/ omitted → payload byte-identical to today (noexpandedkey).{class_type, error: {code: "expand_miss", …}}insideexpanded[]; the search itself never fails.expand_missis registered inerror_codes.REGISTRY(advisory, surfaced in-data).Test evidence
tests/comfy_cli/command/test_nodes_search_expand.py— TDD: captured red first (7/7 fail: unknown option + unregistered code), then green.0byte-identical; zero-matches baseline (expanded: [])expand_missentry, sibling hits still expand); close-match rows expandtest_nodes_introspect.py,test_error_code_registry.py,test_discovery.pygreen; ruff + format clean on touched filesSibling-PR notes
test_op_vocabulary_contract.py; it does not exist on this base and is not claimed here. No overlap with this layer.error_codes.pyadditions never conflict.🤖 Generated with Claude Code