chore: sync the v2 spec and wire every operation it declares - #49
Conversation
A caller holding a job — including one rehydrated by id — had no way to see
the workflow behind it. The SDK holds the graph only when it submitted the
job in the same process; otherwise it is gone.
Calls GET /api/v2/jobs/{id}/workflow and returns both the graph and the
format discriminator. "api" is the executed graph, with editor-only
constructs already resolved away; "save" is the authoring workflow at the
version the job ran. Returning the discriminator matters: a caller must be
able to tell which shape it holds.
Written by hand rather than through the generated client, because the
vendored spec does not describe this endpoint yet. It moves onto the
generated client once the spec re-syncs.
Named to match the existing get_download_url(), and mirrored as getWorkflow()
in the TypeScript SDK.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The required post-sync step: scripts/gen_models.sh against the newly vendored spec, so the drift gate passes. Picks up JobWorkflowResponse and the new job_id/expires_at fields on Asset and Output. Coverage still fails until the two new operations have transport wrappers — getJobWorkflow (#47) and deleteAsset (#41) — since OPERATION_IDS must match the spec exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…/sync-v2-spec-complete
…plete # Conflicts: # spec/openapi.yaml
Registers getJobWorkflow and deleteAsset in OPERATION_IDS/OPERATION_METHODS, moves get_job_workflow onto the generated JobWorkflowResponse model, and fixes six delete-asset test call sites that still passed the base URL positionally after that constructor argument was replaced by COMFY_BASE_URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
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 |
2 participants
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.pyassertsOPERATION_IDSmatches the spec exactly, so a sync that introduces operations without wrappers always fails.The synced spec introduces two:
getJobWorkflowanddeleteAsset. Both are implemented here.What's in it
cloud@7c65b1a), plus regenerated modelsget_workflow()onJob/AsyncJob— returns the graph and aformatdiscriminator (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 controlsAsset.delete()andassets.delete(id)OPERATION_IDS/OPERATION_METHODS, withget_job_workflowmoved onto the generatedJobWorkflowResponsemodelCredit 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.yamlto adddeleteAssetbefore 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-mergedCOMFY_BASE_URLchange, so the two branches were mutually stale — neither PR was wrong alone. Updated toComfy()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 checkall clean; 137 passed, 4 skipped (the skips need a live deployment, as in CI).test_spec_coverage.pypasses 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