[PR #51] [MERGED] docs: cover getWorkflow, asset deletion, and jobId; drop stale JSDoc #62

Closed
opened 2026-10-31 22:04:14 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: wei/readme-v2-surface


📝 Commits (2)

  • 73e2729 docs: cover getWorkflow, asset deletion, and jobId; drop stale JSDoc
  • 3461a19 docs: note the no-recorded-workflow 404 case

📊 Changes

2 files changed (+37 additions, -8 deletions)

View changed files

📝 README.md (+36 -1)
📝 src/sdk/jobs.ts (+1 -7)

📄 Description

@comfyorg/sdk 0.1.7 shipped capabilities the README never mentions, and the source carries two doc comments that stopped being true.

README

  • The workflow behind a job — getWorkflow(), the { workflow, format } shape, and that callers must branch on format. Submissions from this SDK always get "api" today, since v2 has no version-pinning fields yet.
  • client.jobs.get(id) — undocumented anywhere, despite being the whole reason getWorkflow() matters: a rehydrated job has no record of what it ran. Folded into the same section rather than given its own.
  • Asset deletion — asset.delete() (throws on an uncommitted handle) and client.assets.delete(id), with the caveat that it needs a proxy new enough to serve DELETE /api/v2/assets/{id}; older ones return 405.
  • jobId / expiresAt on assets, and jobId on outputs.

Stale JSDoc, which is worse than a stale README

getWorkflow() carried:

Depends on a server endpoint currently in review. Until it ships, expect a 404 … the spec doesn't know this operation yet.

Every clause of that is now false: the endpoint shipped, the spec vendors it, OPERATION_IDS covers it, and two tests exercise it. Unlike a README, this text reaches users as an editor tooltip at the call site, so it actively tells them a working method is broken. Removed.

Also added the missing canceling state to the status doc — the code elsewhere is explicit that canceling is deliberately not terminal, so omitting it from the list a caller reads is a trap.

How it was checked

Every claim was read against the source behind it. The three new snippets were executed against the real client and the project's stub server — submit → poll → getWorkflow, asset commit → delete, and output.jobId — rather than only typechecked.

The rest of the README audited clean: all existing examples, the 12 typed error classes (1:1 with exceptions.ts), the "one method per API operation" claim (11 operations, 11 methods, parity test passing), Node/TypeScript requirements, the auth table, and the hashing claims.

lint, format, typecheck, spec-drift, hygiene, build all clean; 169/169 tests pass.

Noted, not changed

Workflow.fromJson/fromString exist but only fromFile appears in the README. Pre-existing and out of scope here.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Documented committed asset metadata, expiration details, and deletion options.
    • Added guidance for recovering job workflows, supported response formats, SDK submissions, and 404 conditions.
    • Clarified that output handles include the producing job ID.
    • Updated job status documentation to include canceling.
    • Removed outdated workflow endpoint warnings.

🔄 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-typescript-sdk/pull/51 **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/readme-v2-surface` --- ### 📝 Commits (2) - [`73e2729`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/73e272965418d29e540d8dd90a6e09721d0002a5) docs: cover getWorkflow, asset deletion, and jobId; drop stale JSDoc - [`3461a19`](https://github.com/Comfy-Org/comfy-typescript-sdk/commit/3461a1991a8c645fcd0cd98cc057da5b1dec2b15) docs: note the no-recorded-workflow 404 case ### 📊 Changes **2 files changed** (+37 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `README.md` (+36 -1) 📝 `src/sdk/jobs.ts` (+1 -7) </details> ### 📄 Description `@comfyorg/sdk` **0.1.7** shipped capabilities the README never mentions, and the source carries two doc comments that stopped being true. ## README - **The workflow behind a job** — `getWorkflow()`, the `{ workflow, format }` shape, and that callers must branch on `format`. Submissions from this SDK always get `"api"` today, since v2 has no version-pinning fields yet. - **`client.jobs.get(id)`** — undocumented anywhere, despite being the whole reason `getWorkflow()` matters: a rehydrated job has no record of what it ran. Folded into the same section rather than given its own. - **Asset deletion** — `asset.delete()` (throws on an uncommitted handle) and `client.assets.delete(id)`, with the caveat that it needs a proxy new enough to serve `DELETE /api/v2/assets/{id}`; older ones return `405`. - **`jobId` / `expiresAt`** on assets, and `jobId` on outputs. ## Stale JSDoc, which is worse than a stale README `getWorkflow()` carried: > **Depends on a server endpoint currently in review.** Until it ships, expect a 404 … the spec doesn't know this operation yet. Every clause of that is now false: the endpoint shipped, the spec vendors it, `OPERATION_IDS` covers it, and two tests exercise it. Unlike a README, this text reaches users as an **editor tooltip at the call site**, so it actively tells them a working method is broken. Removed. Also added the missing `canceling` state to the `status` doc — the code elsewhere is explicit that `canceling` is deliberately not terminal, so omitting it from the list a caller reads is a trap. ## How it was checked Every claim was read against the source behind it. The three new snippets were executed against the real client and the project's stub server — submit → poll → `getWorkflow`, asset commit → delete, and `output.jobId` — rather than only typechecked. The rest of the README audited clean: all existing examples, the 12 typed error classes (1:1 with `exceptions.ts`), the "one method per API operation" claim (11 operations, 11 methods, parity test passing), Node/TypeScript requirements, the auth table, and the hashing claims. lint, format, typecheck, spec-drift, hygiene, build all clean; **169/169 tests pass**. ## Noted, not changed `Workflow.fromJson`/`fromString` exist but only `fromFile` appears in the README. Pre-existing and out of scope here. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Documented committed asset metadata, expiration details, and deletion options. * Added guidance for recovering job workflows, supported response formats, SDK submissions, and 404 conditions. * Clarified that output handles include the producing job ID. * Updated job status documentation to include `canceling`. * Removed outdated workflow endpoint warnings. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 22:04:14 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Comfy-Org/comfy-typescript-sdk#62
No description provided.