docs: cover get_workflow, asset deletion, and job_id in the README - #51
Conversation
comfy-sdk 0.1.8 shipped three capabilities the README never mentioned. Each addition was checked against the source it describes, and the runnable parts were executed against the installed package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe README adds documentation for asset metadata, asset deletion, workflow retrieval formats, asynchronous equivalents, and output handle ChangesSDK documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Mergeability Score: ⚪ Minimal · up to This localized documentation update adds coverage for existing SDK behavior without changing runtime code or production behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
comfy-sdk0.1.8 shipped three capabilities the README never mentions. A stale reference doc in a published package is worse than a missing one.Added
get_workflow()on both clients, theformatdiscriminator (apivssave), and why this is the only way to see the graph behind a job rehydrated by id. It states plainly that submissions from this SDK always getapitoday, since v2 has no version-pinning fields yet.asset.delete()andclient.assets.delete(id), noting it needs a proxy new enough to serveDELETE /api/v2/assets/{id}; older ones return405.job_idandexpires_at— on assets and outputs, including thatjob_idisNonefor an uploaded asset, which has no producing job.How it was checked
Every claim was read against the source behind it, not the changelog. The runnable parts were executed against the installed package — including confirming the pre-
COMFY_BASE_URLpositional call really does raiseTypeError, since a sibling doc shipped examples that didn't.The rest of the README was audited at the same time and found accurate: install requirements against
pyproject.toml, theCOMFY_BASE_URLand keyword-onlyapi_keybehaviour, the partner-node key distinction,client_info/User-Agent, the typed-exception list againstexceptions.py's map (exact match), the two-layer architecture description, and the release commands against CI.pytest140 passed / 4 skipped; ruff, ruff format, mypy, and the public-repo hygiene check all clean.Two things found but deliberately not changed
Job.cancel()andJob.refresh()have no README mention. Not stale, just absent, and the README already omits other real methods. Worth a decision on whether this doc aims at full method coverage.AsyncOutputhas noto_stream(), whileOutputdoes. That's a code asymmetry rather than a doc bug, so it stayed out of a docs-only change — but it makes the docs-site claim that the sync and async clients have "the same surface" untrue by exactly one method. Filed here so it isn't lost.🤖 Generated with Claude Code
Summary by CodeRabbit