feat: add the typed router exception hierarchy for the models surface

[mattmillerai]

ELI-5

When a model run fails, the server says why in one short machine-readable word — content_policy_violation, provider_timeout, and nine others. Today a Python caller would have to compare that word to a string. This adds one exception class per word, so you write except ContentPolicyViolation: instead. Every one of them is also a RouterError, so you can catch them all at once when you don't care which. And when a newer server sends a word this version has never heard of, you still get a catchable exception with the word on it — the client does not fall over at the exact moment something has already gone wrong.

What this adds

src/comfy_sdk/router_exceptions.py — the typed hierarchy for the client.models surface, plus error_from_response(status, headers, body), which turns a response into the right exception.

The intended class names, so both SDKs implement one list rather than two. The rule is mechanical: the class name is the PascalCase of the wire value, always. test_class_names_are_the_pascal_case_of_the_wire_value asserts it over every class, so a hand-picked name cannot creep in later on either side.

error_type	class	status
invalid_input	InvalidInput	400
content_policy_violation	ContentPolicyViolation	400
provider_error	ProviderError	502
provider_timeout	ProviderTimeout	504
insufficient_credits	InsufficientCredits	402
model_not_found	ModelNotFound	404
unauthorized	Unauthorized	401
forbidden	Forbidden	403
concurrency_limit_exceeded	ConcurrencyLimitExceeded	429
client_disconnected	ClientDisconnected	499
internal_error	InternalError	500

Base is RouterError, which derives from ComfyError, so except ComfyError still covers the whole SDK. The three deferred types (file_download_error, cancelled, queue_timeout) are deliberately absent and a test pins their absence — adding one should be a decision someone makes on purpose, not a constant that quietly widens a set two SDKs generate from.

Per-field validation detail survives as data. A body whose detail is an array becomes ValidationErrorDetail entries on .errors, each with loc (a tuple, integer array indexes kept as ints), msg, type, ctx and input. type is where the specific provider reason lives — image_too_small, file_too_large, missing — which is the granularity the coarse bucket cannot express, so it is an open string rather than an enum. The exception message summarises the entries for a human in addition to, never instead of, the entries themselves.

X-Comfy-Request-Id is on every exception built from a response, as .request_id, so a user reporting a failure has the id to quote.

An unrecognised error_type raises RouterError itself with the raw value on .error_type, rather than failing to decode. Treat one like internal_error; because InternalError is also a RouterError, a caller who wrote the broad catch gets both.

Judgment calls

• The bucket is read off X-Comfy-Error-Type first, the body's error_type second. The per-field validation body carries no error_type of its own, so the header is the only place its bucket appears. Written from one value by one writer server-side, the two cannot normally disagree; the header wins if they ever do.
• A response carrying no bucket at all falls back to its status — but only for statuses that map to exactly one bucket. A proxy or gateway that rejects a call before it reaches the router answers with a status and none of these headers, and except Unauthorized should still fire for a rejected key. 400 is deliberately excluded: it carries either invalid_input or content_policy_violation, and those differ in whether a retry can ever succeed — guessing between them would tell a caller to retry a deterministic refusal. 422 is excluded because the contract pins no bucket to it. Both fall through to RouterError, which is the honest answer. This mirrors the existing comfy_low.errors._CODE_BY_STATUS precedent in this repo.
• .errors lives on the base class, not on one subclass. The bucket a per-field failure carries is read off the header, and the server-side producer for that response is a later story, so the entries have to survive whichever bucket it turns out to be. Nothing in this change depends on guessing it.
• Not re-exported from comfy_sdk/__init__.py, on purpose. Unauthorized, Forbidden and InsufficientCredits already exist there for the workflow surface. One name cannot be two classes, and quietly redefining comfy_sdk.Unauthorized would change what an existing except clause catches. Import the router ones from comfy_sdk.router_exceptions, or catch ComfyError to cover both surfaces. A test pins that the two families stay distinct. If the preference is for a comfy_sdk.models sub-namespace instead, that is a one-line move and worth settling before the first release that ships it.
• error_from_response never raises. Every field is tolerated missing or wrongly-typed, because this code runs while already handling a failure — dropping the two fields that did arrive because a third was malformed helps nobody. 12 malformed-body shapes are tested, including None, an HTML string, b"", a bare list, and a detail[] entry whose every field has the wrong type.
• No retry metadata. Retry behaviour keyed on exception type is a separate story, and the contract declares no Retry-After on the 429, so nothing was invented here.

What is deliberately NOT here

This hierarchy is not yet wired to a call site, because there is no router call to wire it to. client.models landed in #69 as the namespace plus a read-only view of the host client's configuration; it has no run() yet. error_from_response is therefore reachable only from tests today, and the story that adds the model-run call is the one that must route its error responses through it. That is the shape the ticket asks for — the exception hierarchy lands first, generated against the server-side mapping — but it is the one thing a reviewer should not assume is covered.

Verification of the name list — the part that could not be taken on faith

The whole value of these names is that they match the server's set exactly, and a wrong string fails silently: every exception would degrade to the base class and no caller's except clause would ever fire. So the eleven values were not inferred from the story text. They were read off the authoritative server-side error_type declaration and the canonical API contract that defines the two error body shapes and the two response headers, and cross-checked against each other: the six request-level buckets, the five transport-level ones, the three deferred values held out, the per-field entry's loc/msg/type/ctx/input fields, and the status paired with each bucket (which is where this PR's status column and the status-fallback table come from). No value in this change is a guess.

The TypeScript SDK has not implemented its half yet — there is no error_type or router error handling on its main today — so this list is the proposal it should mirror, not a match against an existing one. That cross-check is still open.

Unexercised artifacts

• The two linked design documents are on an internal collaboration tool this environment cannot reach; neither was read.
• No live router endpoint was called. The surface is unreleased and this SDK's vendored spec/openapi.yaml (v2.0.0) does not carry the router routes at all, so scripts/check_drift.py does not cover this module and every test drives a stubbed response rather than a server.
• No sibling story's description was available; only their titles.

Verification

pytest 258 passed / 4 skipped (78 of them new). ruff check, ruff format --check, mypy src, python3 scripts/check_drift.py and python3 scripts/check_public_repo_hygiene.py all clean.

One note for whoever runs the checks locally: uv run ruff … without --extra dev resolves an unpinned ruff, and a newer ruff reformats the Python code blocks inside README.md, which the pinned ruff~=0.15.22 does not touch. That produced an unrelated 24-line README diff here that was reverted. Use uv run --extra dev ruff …, which is what CI installs.

Provenance

• Authored by: agent-work loop
• Verified: pytest: 258 passed, 4 skipped; ruff check: clean; ruff format --check: 46 files already formatted; mypy src: no issues in 18 files; check_drift.py: in sync; check_public_repo_hygiene.py: no internal-only references
• Deviations: the "class names match the TypeScript SDK's exactly" criterion is met only as a proposal — the TypeScript SDK has not implemented its hierarchy yet, so this states the list rather than matching an existing one. The hierarchy is not wired to a call site because the model-run call does not exist yet (see "What is deliberately NOT here"). No other criterion was skipped.

[coderabbitai]

Warning

Review limit reached

Next included review available in 1 minute.

View limit details

Limit details: You’ve used the included review currently available. Your 108 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d012133d-ecd8-4d89-ab2a-da59c9de7621

📥 Commits

Reviewing files that changed from the base of the PR and between 2e6dc46 and b1f4276.

📒 Files selected for processing (2)

• src/comfy_sdk/router_exceptions.py
• tests/test_router_exceptions.py

---

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

[robinjhuang]

Approved — full autonomy check passed.

---

Generated by Claude Code