Skip to content

feat(cli): nodes path --emit-ops (round-trips through apply_specs) - #709

Merged
skishore23 merged 1 commit into
fix/validate-lowers-ui-to-apifrom
kishore/v1-019-emit-ops
Aug 14, 2026
Merged

feat(cli): nodes path --emit-ops (round-trips through apply_specs)#709
skishore23 merged 1 commit into
fix/validate-lowers-ui-to-apifrom
kishore/v1-019-emit-ops

Conversation

@skishore23

@skishore23 skishore23 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Linear: BE-7152 (V1-019) · Layer 3/4 of the V1 hop-killer stack, stacks on #708.

The gap it closes

nodes path reports a plan (a routed node sequence); agents then hand-build the apply batch from it. --emit-ops emits the plan as the batch: each returned path gains paths[].ops — a ready-to-apply spec batch in the frozen edit vocabulary, feedable straight into comfy workflow apply --ops.

Semantics

  • One add_node spec per step with a deterministic dedup-suffixed as: alias (tinysampler, tinysampler_2, … — the same slugging capture_recipe uses, so both surfaces mint identical alias vocabulary).
  • One connect spec per step whose consumed type is produced earlier in the path: from the nearest prior producer's matching output (resolved against the schema, since in --exact mode a step's recorded output_type is only one of possibly several) to the consumer's first link input accepting that type (required inputs first).
  • The path's seed FROM type is produced by nothing in the path, so the first step's input deliberately stays unbound — never a phantom connect.
  • Works in both --exact and --loose modes. Without the flag the payload is byte-identical.

THE CONTRACT IS THE ROUND-TRIP: the emitted specs must pass workflow_ops.apply_specs unchanged. The test feeds them to apply_specs on an empty workflow and asserts the nodes and links exist (including that the minted link's source is the alias-minted node).

$-form note

The vocabulary-freeze PR #704 adds $-sugar for alias references; this base branch predates it, so connect specs reference aliases as bare names — the freeze doc's valid form. Migrating emission to the $-canonical form is a small follow-up once #704 lands.

Test evidence

tests/comfy_cli/command/test_nodes_path_emit_ops.py — TDD: red first (4 fail: unknown option/helper; the byte-identical baseline trivially passes pre-change and pins the invariant after), then green.

  • round-trip through apply_specs on an empty workflow: 2 nodes + 1 link materialize, link source = aliased node
  • alias uniqueness deterministic (tinydecode, tinydecode_2; same input → same output)
  • loose mode emits too; no phantom producers (every connect endpoint is a minted alias)
  • neighbors: test_nodes_introspect.py, test_nodes_search_expand.py, test_workflow_edit.py (150 total) green; ruff clean

Sibling-PR notes

🤖 Generated with Claude Code

nodes path reports a plan; agents then hand-build the apply batch from it.
--emit-ops closes that gap: each returned path gains paths[].ops — a
ready-to-apply spec batch in the frozen edit vocabulary (add_node with as:
aliases + connect referencing them as BARE alias names), feedable straight
into `comfy workflow apply --ops`.

- THE CONTRACT IS THE ROUND-TRIP: tests feed the emitted specs to
  workflow_ops.apply_specs on an empty workflow and assert nodes+links exist.
- deterministic dedup-suffixed aliases (same slugging as capture_recipe).
- each step wires from the NEAREST prior producer of its consumed type,
  resolved against the schema; the seed FROM type stays unbound by design.
- without the flag the payload is byte-identical.
- bare alias refs are the freeze vocabulary's valid form; the $-canonical
  sugar lands with the vocabulary-freeze PR (#704) and is a follow-up here.

Linear: BE-7152

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9f52580b-09c0-4e0c-83ce-df54b207dcad

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Base automatically changed from kishore/v1-018-templates-get to fix/validate-lowers-ui-to-api August 14, 2026 19:32
@skishore23
skishore23 merged commit 03398ad into fix/validate-lowers-ui-to-api Aug 14, 2026
5 checks passed
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.
@skishore23
skishore23 deleted the kishore/v1-019-emit-ops branch August 14, 2026 19:32
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant