[PR #192] feat(errors): expose refusal_subject on RouterError / ContentPolicyViolation #192

New issue

Open

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-17175-refusal-subject

📝 Commits (5)

c8ec6f6 feat(errors): expose refusal_subject on RouterError / ContentPolicyViolation
9db6df3 fix(errors): bound refusal_subject to one printable token
1d07c83 chore: merge main into matt/be-17175-refusal-subject
0372265 Merge remote-tracking branch 'origin/main' into matt/be-17175-refusal-subject
a7c3588 test(router): pin refusal_subject against the synced Router spec

📊 Changes

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

View changed files

📝 CHANGELOG.md (+6 -0)
📝 src/comfy_low/errors.py (+38 -0)
📝 src/comfy_low/transport.py (+3 -0)
📝 src/comfy_sdk/exceptions.py (+1 -0)
📝 src/comfy_sdk/router_exceptions.py (+60 -1)
📝 tests/conftest.py (+9 -1)
📝 tests/test_router_exceptions.py (+184 -0)
📝 tests/test_router_spec_contract.py (+68 -0)

📄 Description

ELI-5

When the Router refuses a model call on content-policy grounds, the SDK raised ContentPolicyViolation but could not tell you what was refused — your prompt, your image, or the model's output. The Router can now say which one (input_image, output_text, ...). This PR reads that and puts it on the exception as exc.refusal_subject.

What changed

RouterError.__init__ gains refusal_subject: str | None = None, stored as .refusal_subject. It is never narrowed to the documented list, the same policy error_type follows, so a subject added after this version was built still reaches the caller. It is bounded, though: every read goes through a shared comfy_low.errors.clean_refusal_subject, which full-matches one token of [A-Za-z0-9_.-]{1,64}. A duplicated header (which httpx joins as "a, b"), control or bidi characters, or an overlong value reads as None instead of reaching a log line verbatim. Same name as the TypeScript SDK's refusalSubject / refusal_subject.
New module constants in comfy_sdk.router_exceptions: REFUSAL_SUBJECT_HEADER = "X-Comfy-Refusal-Subject" and REFUSAL_SUBJECTS, which lists the ten documented values (input, output, input_{text,image,video,audio}, output_{text,image,video,audio}). Both are added to __all__.
error_from_response reads the header first (case-insensitively; a blank or malformed value counts as absent) and falls back to body["refusal_subject"]. A non-string body value becomes None.
The awaited models.run path does not go through error_from_response. It goes transport → comfy_low.ApiError → comfy_sdk.exceptions.to_sdk_error. If only the factory had changed, exc.refusal_subject would always be None on the main call path. So ApiError and error_from_envelope now also accept refusal_subject, using the same header-then-body order. The transport passes the header, and to_sdk_error forwards the value onto the RouterError.
error_from_completion (the queued surface) reads refusal_subject from the completed payload only. The poll and result responses do have headers, but only the body reaches this function, so a subject sent only on the header reads as None on the queued path.
Test stub: two new ServerState knobs, model_run_error_headers and model_run_error_body_extra, for Router-shaped failures.
CHANGELOG [Unreleased] / Added entry.

Tests cover: header wins over body; body only; absent; blank header falls back to body; non-string body value; unknown value passes through unchanged (header and body); malformed values (duplicated, ANSI, newline, bidi, overlong) read as None on the header, body and completion surfaces, and a malformed header falls back to the body; the 422 detail[] validation shape never sets it; the queued completion; and full sync and async models.run runs against the stub server (header-wins, body-only, absent, hostile-header). tests/test_router_spec_contract.py pins REFUSAL_SUBJECTS (values and order) against both places the vendored Router spec states the vocabulary — RouterErrorResponse.refusal_subject and RouterRefusalSubjectHeader — and pins REFUSAL_SUBJECT_HEADER to the name the spec's responses send that header under.

Judgment calls

The ticket asked for the "response → exception factory" only. I also covered the models.run path and the queued completion, for the reason given above.
The attribute is on the base RouterError, as the ticket asked. It is not limited to ContentPolicyViolation. If the Router ever sends the header with another bucket, the value passes through rather than being dropped.

Provenance

Authored by: agent-work loop
Verified:
- At the current head (main cddc298 merged in; the CHANGELOG.md conflict resolved by keeping both main's credits_used note and QueueBacklogFull entry and this PR's entry): ruff check ., ruff format --check . and mypy src are clean.
- pytest: 1294 passed, 9 skipped (the gateway e2e suite skips because no credentials are set).
- The new refusal-subject spec pins fail when the tuple or header name is mutated.
- scripts/check_drift.py and scripts/check_public_repo_hygiene.py: OK.
- PR CI on the current head (a7c3588): all checks green, required and optional.
- The merged result with current main (which adds the cloud@9794097 Router spec sync, #209) passes the same lint, format, mypy and pytest steps, plus both drift checks and the hygiene check.
Deviations:
- None against the ticket's listed changes. The scope is wider than the ticket's "factory only" wording, as described under Judgment calls.
- After review, the value is bounded to one printable token rather than passed through raw.

Residual

Reconciled against the spec. The Router spec sync merged from main (#201) now declares refusal_subject and X-Comfy-Refusal-Subject. The header spelling, the body field name and all ten values (in order) match what this PR shipped, so the tripwire was replaced with the real pins described above. The spec states the vocabulary in prose rather than as an enum, so the pin parses the backticked list after closed vocabulary:; a rewording that moves it fails loudly.
TypeScript SDK parity was not checked. The name follows the stated cross-SDK rule (refusalSubject / refusal_subject), but the TypeScript SDK's routerErrors.ts was not opened.
Not tested against a live Router. The integration e2e suite skips without credentials, so the header/body behaviour was tested only against the in-repo stub server.

Summary by CodeRabbit

New Features
- Router errors now include an optional refusal subject, taken from the response header or error body. A valid header value takes precedence over the body value.
- Refusal subjects are available for failed model runs and refused queued completions. When neither source provides a valid value, the subject is None.
- Unrecognized string subjects are preserved, while blank, malformed, or non-string values are not reported.

_{🔄 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/192 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 9/25/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `matt/be-17175-refusal-subject` --- ### 📝 Commits (5) - [`c8ec6f6`](https://github.com/Comfy-Org/comfy-python-sdk/commit/c8ec6f6f8ba5f76164e10b7c50df81b8019ddbd6) feat(errors): expose refusal_subject on RouterError / ContentPolicyViolation - [`9db6df3`](https://github.com/Comfy-Org/comfy-python-sdk/commit/9db6df337fb3d1110b5cf70e7e93ec3a07e260fe) fix(errors): bound refusal_subject to one printable token - [`1d07c83`](https://github.com/Comfy-Org/comfy-python-sdk/commit/1d07c83836e02aacdbaf5399f3f2bd52c1d10d4f) chore: merge main into matt/be-17175-refusal-subject - [`0372265`](https://github.com/Comfy-Org/comfy-python-sdk/commit/0372265ff2f2b6709706d148c6276ef826694f08) Merge remote-tracking branch 'origin/main' into matt/be-17175-refusal-subject - [`a7c3588`](https://github.com/Comfy-Org/comfy-python-sdk/commit/a7c35888d56691491547eada48edb5d7d1e990d3) test(router): pin refusal_subject against the synced Router spec ### 📊 Changes **8 files changed** (+369 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+6 -0) 📝 `src/comfy_low/errors.py` (+38 -0) 📝 `src/comfy_low/transport.py` (+3 -0) 📝 `src/comfy_sdk/exceptions.py` (+1 -0) 📝 `src/comfy_sdk/router_exceptions.py` (+60 -1) 📝 `tests/conftest.py` (+9 -1) 📝 `tests/test_router_exceptions.py` (+184 -0) 📝 `tests/test_router_spec_contract.py` (+68 -0) </details> ### 📄 Description ## ELI-5 When the Router refuses a model call on content-policy grounds, the SDK raised `ContentPolicyViolation` but could not tell you *what* was refused — your prompt, your image, or the model's output. The Router can now say which one (`input_image`, `output_text`, ...). This PR reads that and puts it on the exception as `exc.refusal_subject`. ## What changed - `RouterError.__init__` gains `refusal_subject: str | None = None`, stored as `.refusal_subject`. It is never narrowed to the documented list, the same policy `error_type` follows, so a subject added after this version was built still reaches the caller. It is bounded, though: every read goes through a shared `comfy_low.errors.clean_refusal_subject`, which full-matches one token of `[A-Za-z0-9_.-]{1,64}`. A duplicated header (which httpx joins as `"a, b"`), control or bidi characters, or an overlong value reads as `None` instead of reaching a log line verbatim. Same name as the TypeScript SDK's `refusalSubject` / `refusal_subject`. - New module constants in `comfy_sdk.router_exceptions`: `REFUSAL_SUBJECT_HEADER = "X-Comfy-Refusal-Subject"` and `REFUSAL_SUBJECTS`, which lists the ten documented values (`input`, `output`, `input_{text,image,video,audio}`, `output_{text,image,video,audio}`). Both are added to `__all__`. - `error_from_response` reads the header first (case-insensitively; a blank or malformed value counts as absent) and falls back to `body["refusal_subject"]`. A non-string body value becomes `None`. - **The awaited `models.run` path does not go through `error_from_response`.** It goes transport → `comfy_low.ApiError` → `comfy_sdk.exceptions.to_sdk_error`. If only the factory had changed, `exc.refusal_subject` would always be `None` on the main call path. So `ApiError` and `error_from_envelope` now also accept `refusal_subject`, using the same header-then-body order. The transport passes the header, and `to_sdk_error` forwards the value onto the `RouterError`. - `error_from_completion` (the queued surface) reads `refusal_subject` from the completed payload only. The poll and result responses do have headers, but only the body reaches this function, so a subject sent only on the header reads as `None` on the queued path. - Test stub: two new `ServerState` knobs, `model_run_error_headers` and `model_run_error_body_extra`, for Router-shaped failures. - CHANGELOG `[Unreleased] / Added` entry. Tests cover: header wins over body; body only; absent; blank header falls back to body; non-string body value; unknown value passes through unchanged (header and body); malformed values (duplicated, ANSI, newline, bidi, overlong) read as `None` on the header, body and completion surfaces, and a malformed header falls back to the body; the 422 `detail[]` validation shape never sets it; the queued completion; and full sync and async `models.run` runs against the stub server (header-wins, body-only, absent, hostile-header). `tests/test_router_spec_contract.py` pins `REFUSAL_SUBJECTS` (values and order) against both places the vendored Router spec states the vocabulary — `RouterErrorResponse.refusal_subject` and `RouterRefusalSubjectHeader` — and pins `REFUSAL_SUBJECT_HEADER` to the name the spec's responses send that header under. ## Judgment calls - The ticket asked for the "response → exception factory" only. I also covered the `models.run` path and the queued completion, for the reason given above. - The attribute is on the base `RouterError`, as the ticket asked. It is not limited to `ContentPolicyViolation`. If the Router ever sends the header with another bucket, the value passes through rather than being dropped. ## Provenance - **Authored by:** agent-work loop - **Verified:** - At the current head (main cddc298 merged in; the `CHANGELOG.md` conflict resolved by keeping both main's `credits_used` note and `QueueBacklogFull` entry and this PR's entry): `ruff check .`, `ruff format --check .` and `mypy src` are clean. - `pytest`: 1294 passed, 9 skipped (the gateway e2e suite skips because no credentials are set). - The new refusal-subject spec pins fail when the tuple or header name is mutated. - `scripts/check_drift.py` and `scripts/check_public_repo_hygiene.py`: OK. - PR CI on the current head (a7c3588): all checks green, required and optional. - The merged result with current main (which adds the cloud@9794097 Router spec sync, #209) passes the same lint, format, mypy and pytest steps, plus both drift checks and the hygiene check. - **Deviations:** - None against the ticket's listed changes. The scope is wider than the ticket's "factory only" wording, as described under Judgment calls. - After review, the value is bounded to one printable token rather than passed through raw. ## Residual - **Reconciled against the spec.** The Router spec sync merged from main (#201) now declares `refusal_subject` and `X-Comfy-Refusal-Subject`. The header spelling, the body field name and all ten values (in order) match what this PR shipped, so the tripwire was replaced with the real pins described above. The spec states the vocabulary in prose rather than as an `enum`, so the pin parses the backticked list after `closed vocabulary:`; a rewording that moves it fails loudly. - **TypeScript SDK parity was not checked.** The name follows the stated cross-SDK rule (`refusalSubject` / `refusal_subject`), but the TypeScript SDK's `routerErrors.ts` was not opened. - **Not tested against a live Router.** The integration e2e suite skips without credentials, so the header/body behaviour was tested only against the in-repo stub server.  ## Summary by CodeRabbit * **New Features** * Router errors now include an optional refusal subject, taken from the response header or error body. A valid header value takes precedence over the body value. * Refusal subjects are available for failed model runs and refused queued completions. When neither source provides a valid value, the subject is `None`. * Unrecognized string subjects are preserved, while blank, malformed, or non-string values are not reported.  --- <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:11 +00:00