feat: add embed_workflow to embed the graph in output metadata - #45
feat: add embed_workflow to embed the graph in output metadata#45wei-hai wants to merge 1 commit into
Conversation
…ut metadata `submit()` and `run()` (sync and async) take a keyword-only `embed_workflow: bool = False`. When set, the SDK sends the materialized graph as `extra_data.extra_pnginfo.workflow` — the key local ComfyUI's SaveImage writes into output PNG metadata — so the graph can be recovered from a generated image when debugging. Off by default, and the wire format is unchanged when off: with no api_key and the flag off, no `extra_data` is sent at all. Merges with the partner-node key when both are given. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough
ChangesWorkflow embedding
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant extra_data_for
participant ComfyAPI
Client->>extra_data_for: Build metadata with materialized workflow
extra_data_for->>ComfyAPI: Submit extra_pnginfo.workflow and optional API key
ComfyAPI-->>Client: Return submitted job
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
wei-hai
left a comment
There was a problem hiding this comment.
Found one blocking workflow-format issue.
| graph = self._materialize(workflow) | ||
| key = idempotency_key or _core.new_idempotency_key() | ||
| extra_data = _core.extra_data_for(api_key) | ||
| extra_data = _core.extra_data_for(api_key, graph if embed_workflow else None) |
There was a problem hiding this comment.
Blocking: graph is API-format, but extra_pnginfo.workflow is consumed by ComfyUI as the editor/save-format graph. A generated PNG already gets the API graph in its prompt chunk from SaveImage; adding this second, API-shaped workflow chunk makes the frontend prioritize it, call loadGraphData() (which expects version/nodes), and return before falling back to prompt, yielding an empty/invalid canvas. Please rely on the existing prompt metadata for API-format recovery, or accept a real UI-format workflow separately / add explicit frontend handling instead of putting graph in this slot.
|
Closing — this approach turns out to be wrong, and the reason is worth recording.
Beyond the mechanics: user feedback reports the workflow recovered from an image is "too mangled to be useful" — Note nodes dropped, Get/Set expanded — because those are editor-only constructs already resolved away by the time the SDK holds the graph. No embedding option recovers them. The real ask is an API that links an output file back to the full original workflow, and one that works for audio, video, 3D and text outputs, which image metadata cannot serve at all. Replacement work will go in that direction. Nothing here is lost — the review findings stay in this thread. |
Recovering the workflow from an image it produced isn't possible today, which makes debugging harder. Adds an opt-in flag.
submit()andrun()— sync and async — take a keyword-onlyembed_workflow: bool = False. When set, the SDK sends the materialized graph asextra_data.extra_pnginfo.workflow, the key local ComfyUI'sSaveImagewrites into output PNG metadata.Behavior
api_keyand the flag off, the wire format is byte-identical to today: noextra_dataat all, never{}. Existing tests pinning this are untouched.workflow— not the caller's input. A test asserts thecore/ASSETsubstitution actually happened, so it can't pass by embedding the wrong object.Reviewer notes
spec/openapi.yamlis untouched by design (synced one-way); it picks the field up on the next sync.embedWorkflow, producing identical bytes.ruff,mypy,pytest(133 passed, 4 pre-existing skips), spec-drift and hygiene checks all clean.🤖 Generated with Claude Code