[PR #49] [MERGED] chore: sync the v2 spec and wire every operation it declares #57

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: wei/sync-v2-spec-complete

📝 Commits (9)

3c2b9e2 feat: add asset delete
1317e5f address coderabbit PR comments
7de4ad0 address coderabbit PR comments
ee2155d feat: add get_workflow() to fetch the graph that produced a job
bec7fb8 chore: sync vendored Comfy API v2 spec from cloud@7c65b1a
24e2c44 chore: regenerate comfy_low models for the synced spec
9c3b078 Merge remote-tracking branch 'origin/wei/fetch-job-workflow' into wei/sync-v2-spec-complete
ba0ed19 Merge remote-tracking branch 'origin/pr-41' into wei/sync-v2-spec-complete
47c46d2 chore: wire the synced spec's new operations into comfy_low

📊 Changes

12 files changed (+478 additions, -25 deletions)

View changed files

📝 spec/openapi.yaml (+134 -16)
📝 src/comfy_low/__init__.py (+4 -0)
📝 src/comfy_low/models/__init__.py (+4 -0)
📝 src/comfy_low/models/_generated.py (+53 -6)
📝 src/comfy_low/transport.py (+29 -1)
📝 src/comfy_sdk/__init__.py (+2 -1)
📝 src/comfy_sdk/assets.py (+26 -0)
📝 src/comfy_sdk/jobs.py (+39 -1)
📝 tests/conftest.py (+42 -0)
📝 tests/test_assets.py (+34 -0)
📝 tests/test_async.py (+33 -0)
➕ tests/test_job_workflow.py (+78 -0)

📄 Description

Brings the vendored Comfy API v2 spec up to date and leaves CI green, which the automated sync PR can't do alone: tests/test_spec_coverage.py asserts OPERATION_IDS matches the spec exactly, so a sync that introduces operations without wrappers always fails.

The synced spec introduces two: getJobWorkflow and deleteAsset. Both are implemented here.

What's in it

Vendored spec synced from the canonical contract (cloud@7c65b1a), plus regenerated models
get_workflow() on Job/AsyncJob — returns the graph and a format discriminator (save = the authoring workflow at the version the job ran; api = the executed graph). Callers must branch on it: which shape comes back depends on how the job was submitted, not on anything the caller controls
Asset deletion — Asset.delete() and assets.delete(id)
Both registered in OPERATION_IDS/OPERATION_METHODS, with get_job_workflow moved onto the generated JobWorkflowResponse model

Credit and supersession

The asset-deletion work is @jab416171's, from #41 — merged here with their commits intact rather than reimplemented, because the spec sync can't go green without it. That PR is superseded by this one; the implementation and tests are theirs.

Their branch had hand-edited spec/openapi.yaml to add deleteAsset before the sync existed. The canonical projection now declares it properly, so the synced file won that conflict — no behaviour change.

One real bug fixed along the way

Six delete-asset tests still called Comfy(server.base_url) positionally. That constructor argument was removed by the separately-merged COMFY_BASE_URL change, so the two branches were mutually stale — neither PR was wrong alone. Updated to Comfy() with the fixture's env var, matching every other test in those files. No test intent, assertions, or delete logic touched.

Verification

ruff, ruff format, mypy, drift, hygiene, and build+twine check all clean; 137 passed, 4 skipped (the skips need a live deployment, as in CI). test_spec_coverage.py passes all four assertions — all 11 spec operations have wrappers on both the sync and async transports.

TypeScript counterpart: Comfy-Org/comfy-typescript-sdk#49.

🤖 Generated with Claude Code

_{🔄 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/49 **Author:** [@wei-hai](https://github.com/wei-hai) **Created:** 8/13/2026 **Status:** ✅ Merged **Merged:** 8/13/2026 **Merged by:** [@wei-hai](https://github.com/wei-hai) **Base:** `main` ← **Head:** `wei/sync-v2-spec-complete` --- ### 📝 Commits (9) - [`3c2b9e2`](https://github.com/Comfy-Org/comfy-python-sdk/commit/3c2b9e2e728b5e245dfcb6eda17453953f9c2bc5) feat: add asset delete - [`1317e5f`](https://github.com/Comfy-Org/comfy-python-sdk/commit/1317e5f5a67bba83663f22cb1e6cc486b5c0abef) address coderabbit PR comments - [`7de4ad0`](https://github.com/Comfy-Org/comfy-python-sdk/commit/7de4ad0e2125f36ba7a21c700eaeebf4eb00da9d) address coderabbit PR comments - [`ee2155d`](https://github.com/Comfy-Org/comfy-python-sdk/commit/ee2155d984d9cc946e54382bf5ae252ad22e9d2d) feat: add get_workflow() to fetch the graph that produced a job - [`bec7fb8`](https://github.com/Comfy-Org/comfy-python-sdk/commit/bec7fb8e609569f0d120e34cabdc82aa15ad1c12) chore: sync vendored Comfy API v2 spec from cloud@7c65b1a - [`24e2c44`](https://github.com/Comfy-Org/comfy-python-sdk/commit/24e2c448dc9cbffea97064e7b92ff262ac028b96) chore: regenerate comfy_low models for the synced spec - [`9c3b078`](https://github.com/Comfy-Org/comfy-python-sdk/commit/9c3b0780efca85f797593e1a5def94721b12cfae) Merge remote-tracking branch 'origin/wei/fetch-job-workflow' into wei/sync-v2-spec-complete - [`ba0ed19`](https://github.com/Comfy-Org/comfy-python-sdk/commit/ba0ed1933ed224a1899c711ec08148b541cbdb39) Merge remote-tracking branch 'origin/pr-41' into wei/sync-v2-spec-complete - [`47c46d2`](https://github.com/Comfy-Org/comfy-python-sdk/commit/47c46d29e3d0797f7aaa4c203fe3c07402d71e6d) chore: wire the synced spec's new operations into comfy_low ### 📊 Changes **12 files changed** (+478 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `spec/openapi.yaml` (+134 -16) 📝 `src/comfy_low/__init__.py` (+4 -0) 📝 `src/comfy_low/models/__init__.py` (+4 -0) 📝 `src/comfy_low/models/_generated.py` (+53 -6) 📝 `src/comfy_low/transport.py` (+29 -1) 📝 `src/comfy_sdk/__init__.py` (+2 -1) 📝 `src/comfy_sdk/assets.py` (+26 -0) 📝 `src/comfy_sdk/jobs.py` (+39 -1) 📝 `tests/conftest.py` (+42 -0) 📝 `tests/test_assets.py` (+34 -0) 📝 `tests/test_async.py` (+33 -0) ➕ `tests/test_job_workflow.py` (+78 -0) </details> ### 📄 Description Brings the vendored Comfy API v2 spec up to date **and leaves CI green**, which the automated sync PR can't do alone: `tests/test_spec_coverage.py` asserts `OPERATION_IDS` matches the spec exactly, so a sync that introduces operations without wrappers always fails. The synced spec introduces two: `getJobWorkflow` and `deleteAsset`. Both are implemented here. ## What's in it - **Vendored spec** synced from the canonical contract (`cloud@7c65b1a`), plus regenerated models - **`get_workflow()`** on `Job`/`AsyncJob` — returns the graph *and* a `format` discriminator (`save` = the authoring workflow at the version the job ran; `api` = the executed graph). Callers must branch on it: which shape comes back depends on how the job was submitted, not on anything the caller controls - **Asset deletion** — `Asset.delete()` and `assets.delete(id)` - Both registered in `OPERATION_IDS`/`OPERATION_METHODS`, with `get_job_workflow` moved onto the generated `JobWorkflowResponse` model ## Credit and supersession The asset-deletion work is **@jab416171's**, from #41 — merged here with their commits intact rather than reimplemented, because the spec sync can't go green without it. That PR is superseded by this one; the implementation and tests are theirs. Their branch had hand-edited `spec/openapi.yaml` to add `deleteAsset` before the sync existed. The canonical projection now declares it properly, so the synced file won that conflict — no behaviour change. ## One real bug fixed along the way Six delete-asset tests still called `Comfy(server.base_url)` positionally. That constructor argument was removed by the separately-merged `COMFY_BASE_URL` change, so the two branches were mutually stale — neither PR was wrong alone. Updated to `Comfy()` with the fixture's env var, matching every other test in those files. No test intent, assertions, or delete logic touched. ## Verification ruff, ruff format, mypy, drift, hygiene, and `build`+`twine check` all clean; **137 passed, 4 skipped** (the skips need a live deployment, as in CI). `test_spec_coverage.py` passes all four assertions — all 11 spec operations have wrappers on both the sync and async transports. TypeScript counterpart: Comfy-Org/comfy-typescript-sdk#49. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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:04 +00:00