[PR #52] [MERGED] docs: document from_json/from_str and kind-typed outputs in the README #59

New issue

Closed

opened 2026-10-31 22:01:05 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:01:05 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-python-sdk/pull/52
Author: @mattmillerai
Created: 8/13/2026
Status: ✅ Merged
Merged: 8/24/2026
Merged by: @wei-hai

Base: main ← Head: matt/be-7455-readme-from-json-and-typed-outputs

📝 Commits (1)

7278496 docs: document from_json/from_str and kind-typed outputs in the README

📊 Changes

1 file changed (+47 additions, -2 deletions)

View changed files

📝 README.md (+47 -2)

📄 Description

ELI-5

The README only ever showed one way to build a workflow — load workflow_api.json off disk — and only ever showed saving an image to a file. The SDK can do more than that: you can hand it a graph you built in code, and outputs tell you whether they're an image, a video, audio or text so you can pull them as raw bytes or as a link instead of a file. None of that was written down, so anyone integrating had to read the SDK source to find it. This writes it down. Docs only — no code changed.

What changed

Getting started → new "Constructing a workflow" subsection. A three-row table covering from_file(path) / from_json(graph) / from_str(text), a note that all three are local (no network I/O), and a worked example that builds a graph as a dict, calls set_input on it, submits with client.run, and pulls the result with to_bytes() — the headless/cron shape with no filesystem round-trip. It also states that from_json wraps the dict rather than copying it (so wf.json is your graph and stays freely mutable), and that AsyncComfy exposes the same three constructors.

Downloading outputs → new "Outputs are kind-typed" subsection. output.type is documented as the normalized kind, alongside content_type / name / size_bytes, with an example that branches across kinds and uses a different retrieval path for each: to_file for image, to_bytes for audio, get_download_url() for video, and a case _ fallback. to_bytes() and get_download_url() themselves were already documented (#50/#51); what was missing was the kind discriminator and any non-image example — before this, the README's only mentions of audio or video were zero.

Verification

ruff check ., ruff format --check ., mypy src, pytest — all green (140 passed, 4 skipped; the skips are the pre-existing integration tests).

Beyond the suite, every claim the new prose makes was exercised rather than asserted. I wrote a throwaway pytest module that runs the new snippets verbatim against the repo's own stub server and deleted it before committing (this is a docs-only PR, so it is not in the diff). It confirmed: from_json wraps without copying (wf.json is graph), set_input mutates that same dict, from_str round-trips the JSON text, to_bytes() returns the served bytes, and a job carrying four outputs typed image / audio / video / text dispatches correctly through the match in the example, including get_download_url().url.

The same names were then checked against the published 0.1.8 release (not just this tree), since the acceptance bar is that the examples work on the current published 0.1.x: WorkflowFactory has all three constructors, Output has to_file / to_stream / to_bytes / get_download_url / type / content_type / name / size_bytes, and OutputType is the same six-value enum.

Judgment calls

Six kinds, not four. The brief I worked from named the kinds as image | video | audio | text. The source of truth — the OutputType enum generated from spec/openapi.yaml — has six: image, video, audio, text, file, latent. I documented the six that exist. The case _ fallback in the example is deliberate for the same reason: the README isn't covered by the codegen drift check, so a reader who copies the snippet still handles a kind added later.

The example graph is abridged and says so. A genuinely runnable API-format graph is roughly seven nodes of checkpoint/CLIP/latent plumbing, which would bury the point of the snippet. It is labelled (abridged); node 9 references a node 8 that isn't shown.

Not covered by this PR

Output.to_stream() is still undocumented, deliberately. It has no async twin — AsyncOutput.to_stream does not exist on 0.1.8 or on this tree — and the outputs section closes with "AsyncOutput mirrors all of the above with await", which documenting it there would make false. Writing it up properly means either adding the async method or carving out an exception, and both are outside a docs-only change.
Five other public members remain unmentioned in the README. Sizing the portion this PR does not cover: a word-boundary text sweep of every public method and property on the comfy_sdk classes against the README says 6 of 41 distinct names go unmentioned after this change (aclose, as_reference, close, created_new, refresh, and the to_stream above), down from 10 before it (content_type, from_json, from_str and size_bytes are the four this PR adds). The sweep is coarse — it matches bare words, so a name like type counts as mentioned wherever the English word appears — but it is symmetric across both revisions.
Two artifacts named by my source task could not be read from this environment and are therefore unexercised: an internal tracking issue for a downstream Python consumer migration (the case that motivated this), and a sibling issue filed against the TypeScript SDK. Both live on an internal tracker with no access from here, so I could not confirm whether either constrains the wording beyond what's above. Nothing in this diff depends on them — it is grounded in src/comfy_sdk/workflows.py, src/comfy_sdk/outputs.py and the generated OutputType — but they are named here rather than assumed away.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-python-sdk/pull/52 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 8/13/2026 **Status:** ✅ Merged **Merged:** 8/24/2026 **Merged by:** [@wei-hai](https://github.com/wei-hai) **Base:** `main` ← **Head:** `matt/be-7455-readme-from-json-and-typed-outputs` --- ### 📝 Commits (1) - [`7278496`](https://github.com/Comfy-Org/comfy-python-sdk/commit/7278496c8f748a0482cd424bc24deee3030a0d73) docs: document from_json/from_str and kind-typed outputs in the README ### 📊 Changes **1 file changed** (+47 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+47 -2) </details> ### 📄 Description ## ELI-5 The README only ever showed one way to build a workflow — load `workflow_api.json` off disk — and only ever showed saving an image to a file. The SDK can do more than that: you can hand it a graph you built in code, and outputs tell you whether they're an image, a video, audio or text so you can pull them as raw bytes or as a link instead of a file. None of that was written down, so anyone integrating had to read the SDK source to find it. This writes it down. Docs only — no code changed. ## What changed **Getting started → new "Constructing a workflow" subsection.** A three-row table covering `from_file(path)` / `from_json(graph)` / `from_str(text)`, a note that all three are local (no network I/O), and a worked example that builds a graph as a `dict`, calls `set_input` on it, submits with `client.run`, and pulls the result with `to_bytes()` — the headless/cron shape with no filesystem round-trip. It also states that `from_json` wraps the dict rather than copying it (so `wf.json` *is* your graph and stays freely mutable), and that `AsyncComfy` exposes the same three constructors. **Downloading outputs → new "Outputs are kind-typed" subsection.** `output.type` is documented as the normalized kind, alongside `content_type` / `name` / `size_bytes`, with an example that branches across kinds and uses a different retrieval path for each: `to_file` for image, `to_bytes` for audio, `get_download_url()` for video, and a `case _` fallback. `to_bytes()` and `get_download_url()` themselves were already documented (#50/#51); what was missing was the kind discriminator and any non-image example — before this, the README's only mentions of audio or video were zero. ## Verification `ruff check .`, `ruff format --check .`, `mypy src`, `pytest` — all green (140 passed, 4 skipped; the skips are the pre-existing integration tests). Beyond the suite, every claim the new prose makes was exercised rather than asserted. I wrote a throwaway pytest module that runs the new snippets verbatim against the repo's own stub server and deleted it before committing (this is a docs-only PR, so it is not in the diff). It confirmed: `from_json` wraps without copying (`wf.json is graph`), `set_input` mutates that same dict, `from_str` round-trips the JSON text, `to_bytes()` returns the served bytes, and a job carrying four outputs typed `image` / `audio` / `video` / `text` dispatches correctly through the `match` in the example, including `get_download_url().url`. The same names were then checked against the **published 0.1.8** release (not just this tree), since the acceptance bar is that the examples work on the current published 0.1.x: `WorkflowFactory` has all three constructors, `Output` has `to_file` / `to_stream` / `to_bytes` / `get_download_url` / `type` / `content_type` / `name` / `size_bytes`, and `OutputType` is the same six-value enum. ## Judgment calls **Six kinds, not four.** The brief I worked from named the kinds as `image | video | audio | text`. The source of truth — the `OutputType` enum generated from `spec/openapi.yaml` — has six: `image`, `video`, `audio`, `text`, `file`, `latent`. I documented the six that exist. The `case _` fallback in the example is deliberate for the same reason: the README isn't covered by the codegen drift check, so a reader who copies the snippet still handles a kind added later. **The example graph is abridged and says so.** A genuinely runnable API-format graph is roughly seven nodes of checkpoint/CLIP/latent plumbing, which would bury the point of the snippet. It is labelled `(abridged)`; node `9` references a node `8` that isn't shown. ## Not covered by this PR - **`Output.to_stream()` is still undocumented, deliberately.** It has no async twin — `AsyncOutput.to_stream` does not exist on 0.1.8 or on this tree — and the outputs section closes with "`AsyncOutput` mirrors all of the above with `await`", which documenting it there would make false. Writing it up properly means either adding the async method or carving out an exception, and both are outside a docs-only change. - **Five other public members remain unmentioned in the README.** Sizing the portion this PR does *not* cover: a word-boundary text sweep of every public method and property on the `comfy_sdk` classes against the README says **6 of 41 distinct names go unmentioned after this change** (`aclose`, `as_reference`, `close`, `created_new`, `refresh`, and the `to_stream` above), **down from 10 before it** (`content_type`, `from_json`, `from_str` and `size_bytes` are the four this PR adds). The sweep is coarse — it matches bare words, so a name like `type` counts as mentioned wherever the English word appears — but it is symmetric across both revisions. - **Two artifacts named by my source task could not be read from this environment and are therefore unexercised**: an internal tracking issue for a downstream Python consumer migration (the case that motivated this), and a sibling issue filed against the TypeScript SDK. Both live on an internal tracker with no access from here, so I could not confirm whether either constrains the wording beyond what's above. Nothing in this diff depends on them — it is grounded in `src/comfy_sdk/workflows.py`, `src/comfy_sdk/outputs.py` and the generated `OutputType` — but they are named here rather than assumed away. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 22:01:05 +00:00