[mattmillerai]

ELI-5

When the server tells you "slow down, you've used up your request allowance for now" (rate_limited), the SDK already raised a RateLimited exception, but you could only import it from comfy_sdk.router_exceptions. Now from comfy_sdk import RateLimited works too, the same way Unauthorized, Forbidden and InsufficientCredits already do. It is the same class object, so an except written against either import catches the error.

What changed

• comfy_sdk.exceptions: RateLimited (re-exported from router_exceptions, the same object) is listed explicitly in _BY_CODE as "rate_limited", beside insufficient_credits. It is also added to __all__. The comments and docstrings that counted "three" shared buckets now say four.
• comfy_sdk root: imports and exports RateLimited. The __all__ comment now says that the four shared buckets are lifted and the Router-only buckets stay in router_exceptions.
• comfy_sdk.router_exceptions: only the module docstring changed, to count four shared names and to describe what the package root exports. The class itself is unchanged.
• comfy_low.errors: adds class RateLimited(ApiError): code = "rate_limited", with a docstring that contrasts it with queue_full, and adds it to _BY_CODE. _CODE_BY_STATUS[429] = "queue_full" is unchanged, so a 429 with no code still decodes as QueueFull.
• Tests:
  • test_exception_modules.py: the shared-name and descends-from-both checks now include RateLimited.
  • test_error_mapping.py: rate_limited moved from the Router-only parametrize list to test_a_bucket_both_surfaces_spell_keeps_its_v2_class. Two new tests:
    • SDK: to_sdk_error(ApiError("slow down", code="rate_limited", http_status=429, retry_after=3)) returns an instance of comfy_sdk.RateLimited, which is the same object as exceptions.RateLimited and router_exceptions.RateLimited. retry_after is 3.
    • Low layer: error_from_envelope(429, {"error": {"code": "rate_limited", ...}}, retry_after=3) returns a comfy_low.errors.RateLimited, and a 429 with no code still returns QueueFull.
  • test_job_metadata.py now imports RateLimited from comfy_sdk, which proves the root export through the existing pytest.raises cases.
• Docs: the README typed-errors list gains a RateLimited entry. It says .retry_after is None when the response sent no delta-seconds Retry-After, because unlike QueueFull's it is not defaulted to 0. The comfy_low/errors.py comments that counted three identically spelled codes now count four. The Router section's shared-names paragraph now counts four. CHANGELOG has a new entry under Unreleased → Added.

Judgment calls

• comfy_low root export. I also exported RateLimited from the comfy_low package root. Every other class in comfy_low.errors is exported there, and the TypeScript low layer is meant to mirror this one.
• Low-layer behaviour change. A Router rate_limited response, or a v2 one, now becomes comfy_low.errors.RateLimited at the low layer instead of a bare ApiError. It is still an ApiError subclass, and I found nothing in src/ that checks type(exc) is ApiError or relies on that code being untyped.
• No SDK-level behaviour change. _class_for already resolved rate_limited to this class through the _BY_ERROR_TYPE fallback. Listing it in _BY_CODE documents the mapping rather than changing it, so removing the _BY_CODE entry would not fail the new SDK test. The test pins the outcome, not which lookup produced it.

Provenance

• Authored by: agent-work loop
• Verified: (on head da7a435, after merging main at 6d2dd72)
  • Tests are green: pytest -q gives 1267 passed and 9 skipped. The skips include the env-gated gateway e2e.
  • ruff check ., ruff format --check . and mypy src are clean.
  • scripts/check_drift.py is OK: models are in sync, and the 19 Router error types are covered and blessed.
  • scripts/check_public_repo_hygiene.py is OK.
  • Not run locally: the 3.10–3.13 matrix (only one interpreter was used) and build-check. CI runs both on push.
• Deviations: none from the plan, apart from the extra comfy_low root export described above. The CHANGELOG entry sits under a fresh Unreleased → Added, because main cut 0.5.0 while this PR was open.

Residual

• Overlap with open PRs. The open PR that maps sso_required → Forbidden (feat(errors): map sso_required to Forbidden and carry organization_id #205) also edits src/comfy_sdk/exceptions.py and src/comfy_low/errors.py. Whichever lands second needs a rebase, which should be mechanical: different _BY_CODE entries and adjacent comment lines. The HEAD-429 rate_limited PR (fix(assets): read a bodiless HEAD 429 on the by-hash probe as rate_limited, not queue_full #208) edits only comfy_low/transport.py. Its bodiless HEAD probe will produce the new comfy_low.errors.RateLimited / comfy_sdk.RateLimited once both are merged, but I did not exercise that combination here.
• TypeScript twin. The TypeScript SDK's root RateLimited export, and its surface-parity.test.ts check, are a separate follow-up that this PR unblocks. That repo was not touched or run here.

Summary by CodeRabbit

• New Features
  • Added a shared RateLimited exception for rate-limit responses, available from the SDK and low-level package.
  • Rate-limit errors include their retry delay and HTTP status. HTTP 429 responses without a rate-limit code continue to map to QueueFull.
• Documentation
  • Clarified how RateLimited relates to other workflow and Router errors, and documented its retry_after attribute.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: Comfy-Org/comfy-python-sdk/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Team
• Run ID: 628f99ab-ed53-41a9-b01f-913554b64055

---

---

---

📥 Commits

Reviewing files that changed from the base of the PR and between 602b405 and da7a435.

---

---

---

📒 Files selected for processing (8)

• CHANGELOG.md
• README.md
• src/comfy_low/__init__.py
• src/comfy_low/errors.py
• src/comfy_sdk/__init__.py
• src/comfy_sdk/exceptions.py
• src/comfy_sdk/router_exceptions.py
• tests/test_job_metadata.py

---

---

---

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

---

---

---

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

The change adds RateLimited as a typed low-level error and exports it through comfy_low. The SDK shares this exception across its public modules and maps rate_limited responses to it. Tests cover the mapping, exports, inheritance, status, and retry hint.

Changes

RateLimited exception

Layer / File(s)	Summary
Add the low-level typed error src/comfy_low/errors.py, src/comfy_low/__init__.py	RateLimited is added as an ApiError subclass and mapped from the rate_limited code. The low-level package exports it.
Share the SDK exception across routes src/comfy_sdk/..., tests/test_error_mapping.py, tests/test_exception_modules.py, tests/test_job_metadata.py, README.md, CHANGELOG.md	The SDK exports RateLimited from its public modules and maps the shared code to it. Tests check its shared class identity, inheritance, status, and retry hint. Documentation describes the shared exception and distinguishes its time-window limit from QueueFull backpressure.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: wei-hai, deepme987

---

---

---

---

Merge Risk: ⚪ Minimal · up to da7a4

Coded rate-limit responses use the new shared exception, and the bare-429 fallback and retry hint behavior are documented. No merge-blocking issue is identified.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 8 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely identifies the main changes: exporting RateLimited as a shared error and mapping v2 rate_limited responses to it.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 8 files. (2 skipped: 2 unsupported.)

---

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

---

---

---

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

---

---

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 2 finding(s).

Severity	Count
🟢 Low	1
⚪ Nit	1

Panel: 6/6 reviewers contributed findings.