[PR #207] feat(jobs): add JobSummary.release_version and correct the deployment_id meaning #206

New issue

Open

opened 2026-10-31 22:02:19 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:02:19 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-python-sdk/pull/207
Author: @mattmillerai
Created: 10/9/2026
Status: 🔄 Open

Base: main ← Head: mattmiller/be-19947-job-summary-release-version

📝 Commits (2)

f44fc19 feat(jobs): add JobSummary.release_version and correct the deployment_id meaning
9241c1e docs(jobs): say what best-effort means for deployment_id

📊 Changes

4 files changed (+86 additions, -11 deletions)

View changed files

📝 CHANGELOG.md (+6 -2)
📝 README.md (+8 -3)
📝 src/comfy_sdk/jobs.py (+27 -6)
📝 tests/test_job_metadata.py (+45 -0)

📄 Description

ELI-5

When you list jobs, each one now says which release of the deployment ran it (release_version). The note on deployment_id also gets fixed: it is the deployment you sent the job to, and it stays the same when that deployment moves to a new release. It is not the release copy that ran the job.

Summary

JobSummary.release_version: int | None is a new field, placed after deployment_id and before metadata. The dataclass stays frozen=True, slots=True, and _from_item builds it with keyword arguments.
_from_item reads item.get("release_version"). It accepts an int or None. Any other type raises ComfyError(code="invalid_response") naming release_version, the same rule deployment_id already follows. That includes a bool, which is an int subclass and is ruled out on purpose.
The deployment_id docstring, README and CHANGELOG now give the posted-to meaning. Because the v2 JobListItem contract does not declare the field, they also say a host may leave it out, in which case it reads as None. A value that is present but has the wrong type still raises invalid_response, as it did before.
The CHANGELOG [Unreleased] JobSummary bullet lists release_version and the new error case. The README list_jobs section lists the field.
New tests:
- release_version: 3 comes through on both Comfy.list_jobs and AsyncComfy.list_jobs.
- An absent or null value reads as None.
- "3", True and 3.0 each raise invalid_response naming release_version. Each bad-item case runs against both the sync and async clients.

Judgment call (error text): the old message said has a non-string <fields>, which would be wrong for an integer field. Each wrong field now carries its own qualifier, for example job list item has a non-string id and a non-integer release_version. When several fields are wrong, the wording is slightly different from before. The code is still invalid_response, and each field is still named. The existing tests only check that the field name appears in the message, so they pass unchanged.

The change only adds to the API. list_jobs and JobSummary are still unreleased.

Residual

The vendored spec/openapi.yaml in this tree does not declare release_version yet. The integer type comes from the upstream contract change that adds it to JobListItem, which has not been synced here. I did not check the field's type against a synced spec or a live gateway. If the upstream schema turns out not to be integer, the type check in _from_item needs to change to match. A pending spec-sync PR may bring it in. JobSummary is built from the raw dict rather than the generated models, so that sync does not affect this change either way.
tests/integration/test_gateway_e2e.py was skipped because there are no credentials. Nothing here was checked against a live serverless gateway response.
Declaring deployment_id in the upstream v2 contract is out of scope and is up to the API owners.
The TypeScript SDK needs the same change. That is separate work in its own repo.

Provenance

Authored by: agent-work loop
Verified:
- Tests: the full suite passed locally on Python 3.12, 1266 passed and 9 skipped (the skips include the gateway e2e test, which needs credentials). After the review-round docs fix, tests/test_job_metadata.py was re-run and passed (113).
- Lint, format and types: ruff check, ruff format --check and mypy src are clean.
- Public-repo hygiene script: OK.
- CI on the current head 9241c1e is green, including the 3.10 to 3.13 matrix, build-check and codegen-drift.
- Merged with current main (cddc298, a Router spec sync), the result merges cleanly and passes ruff check, ruff format --check, mypy src, and the job-metadata and Router test modules (400 passed).
Deviations: none from the requested change. The only extras are the reworded error text described above and a review-round clarification of what "best-effort" means for deployment_id (docs only).

_{🔄 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/207 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/9/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `mattmiller/be-19947-job-summary-release-version` --- ### 📝 Commits (2) - [`f44fc19`](https://github.com/Comfy-Org/comfy-python-sdk/commit/f44fc19b7829e9ba8d82cb66e4627b3b9845fca7) feat(jobs): add JobSummary.release_version and correct the deployment_id meaning - [`9241c1e`](https://github.com/Comfy-Org/comfy-python-sdk/commit/9241c1e9597c738bedd8d9dd0bfab890b8e06441) docs(jobs): say what best-effort means for deployment_id ### 📊 Changes **4 files changed** (+86 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+6 -2) 📝 `README.md` (+8 -3) 📝 `src/comfy_sdk/jobs.py` (+27 -6) 📝 `tests/test_job_metadata.py` (+45 -0) </details> ### 📄 Description ## ELI-5 When you list jobs, each one now says which release of the deployment ran it (`release_version`). The note on `deployment_id` also gets fixed: it is the deployment you sent the job to, and it stays the same when that deployment moves to a new release. It is not the release copy that ran the job. ## Summary - `JobSummary.release_version: int | None` is a new field, placed after `deployment_id` and before `metadata`. The dataclass stays `frozen=True, slots=True`, and `_from_item` builds it with keyword arguments. - `_from_item` reads `item.get("release_version")`. It accepts an `int` or `None`. Any other type raises `ComfyError(code="invalid_response")` naming `release_version`, the same rule `deployment_id` already follows. That includes a `bool`, which is an `int` subclass and is ruled out on purpose. - The `deployment_id` docstring, README and CHANGELOG now give the posted-to meaning. Because the v2 `JobListItem` contract does not declare the field, they also say a host may leave it out, in which case it reads as `None`. A value that is present but has the wrong type still raises `invalid_response`, as it did before. - The CHANGELOG `[Unreleased]` `JobSummary` bullet lists `release_version` and the new error case. The README `list_jobs` section lists the field. - New tests: - `release_version: 3` comes through on both `Comfy.list_jobs` and `AsyncComfy.list_jobs`. - An absent or null value reads as `None`. - `"3"`, `True` and `3.0` each raise `invalid_response` naming `release_version`. Each bad-item case runs against both the sync and async clients. **Judgment call (error text):** the old message said `has a non-string <fields>`, which would be wrong for an integer field. Each wrong field now carries its own qualifier, for example `job list item has a non-string id and a non-integer release_version`. When several fields are wrong, the wording is slightly different from before. The code is still `invalid_response`, and each field is still named. The existing tests only check that the field name appears in the message, so they pass unchanged. The change only adds to the API. `list_jobs` and `JobSummary` are still unreleased. ## Residual - The vendored `spec/openapi.yaml` in this tree does not declare `release_version` yet. The integer type comes from the upstream contract change that adds it to `JobListItem`, which has not been synced here. I did not check the field's type against a synced spec or a live gateway. If the upstream schema turns out not to be `integer`, the type check in `_from_item` needs to change to match. A pending spec-sync PR may bring it in. `JobSummary` is built from the raw dict rather than the generated models, so that sync does not affect this change either way. - `tests/integration/test_gateway_e2e.py` was skipped because there are no credentials. Nothing here was checked against a live serverless gateway response. - Declaring `deployment_id` in the upstream v2 contract is out of scope and is up to the API owners. - The TypeScript SDK needs the same change. That is separate work in its own repo. ## Provenance - **Authored by:** agent-work loop - **Verified:** - Tests: the full suite passed locally on Python 3.12, 1266 passed and 9 skipped (the skips include the gateway e2e test, which needs credentials). After the review-round docs fix, `tests/test_job_metadata.py` was re-run and passed (113). - Lint, format and types: `ruff check`, `ruff format --check` and `mypy src` are clean. - Public-repo hygiene script: OK. - CI on the current head `9241c1e` is green, including the 3.10 to 3.13 matrix, `build-check` and `codegen-drift`. - Merged with current `main` (`cddc298`, a Router spec sync), the result merges cleanly and passes `ruff check`, `ruff format --check`, `mypy src`, and the job-metadata and Router test modules (400 passed). - **Deviations:** none from the requested change. The only extras are the reworded error text described above and a review-round clarification of what "best-effort" means for `deployment_id` (docs only). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 22:02:19 +00:00

bsakaguchi referenced this issue

2026-10-31 22:02:19 +00:00

[PR #206] fix(events): raise a typed error on the terminal SSE error frame instead of reconnecting #207