fix(errors): sanitise and unify the Router detail[] validation summary

[mattmillerai]

ELI-5

When a model call is rejected for a bad field, the server sends back a little list of what went wrong. The SDK turns that list into the human-readable error message.

Two problems: (1) it pasted the server's text straight in without cleaning it, so a message containing a newline, a hidden control byte, or an ANSI escape could scribble on your terminal or flood a log line — and a big list could make the message enormous. (2) The message you got from models.run() looked different from the one you got on the queued/submit() path for the exact same server response: run() dropped the field names, so two field required errors collapsed to a useless field required; field required, while the queued path told you which two fields.

This change cleans the text (same 256-char, single-line, no-control-chars treatment every other body-derived string already gets) and makes both paths build the message with one shared function, so one server response always reads the same way.

What changed

• New comfy_low.summarise_detail() renders each detail[] entry as <loc>: <msg> and runs the joined line through clean_body_excerpt (unprintable-category reduction, whitespace collapse, _BODY_EXCERPT_LIMIT/256 cap). Both error_from_envelope (awaited run() path) and router_exceptions (queued path) now summarise the array through this one function, so a single wire body yields a single .detail. The now-redundant _summarise in router_exceptions is removed.
• Bounded accumulation. The summariser stops appending once it has reached the _BODY_EXCERPT_LIMIT * _BODY_EXCERPT_WINDOW prefix clean_body_excerpt actually reads, rather than rendering a server-controlled array in full and slicing 256 characters off the end — so describing a body costs the same whether the body is small or huge.
• loc members that are not path segments are dropped, not coerced. loc is typed tuple[str | int, ...] — field names and array indices — so a nested, server-controlled member is skipped instead of being str()-ed into a Python repr in the user-visible message. This applied in two places: the summariser, and ValidationErrorDetail, whose _detail_from was storing the repr and whose .location re-rendered it. .location now delegates to the one shared renderer, so the summary and .errors[i].location cannot disagree about what a field is called.
• _clean docstring corrected: the list detail shape still reaches _clean through the message = _clean(raw_detail) fallback (a validation array that produced no summary), so its isinstance(value, str) guard is load-bearing — it keeps a Python list repr out of the caller's message. The old docstring claimed the shape never reaches the function, which invited deleting the guard.

Each item has a regression test; the run-vs-queued .detail parity is asserted (assert excinfo.value.detail == queued.detail).

Dropped after review: the to_sdk_error reroute

An earlier revision made a detail[] array select a RouterError even when the code collided with a v2 envelope bucket. That is removed, for two independent reasons:

1. main solved it properly. fix(errors): make except RouterError catch every Router refusal #157 merged the two exception hierarchies, so unauthorized/forbidden/insufficient_credits are one RouterError subclass each. For any response carrying real Router provenance, _class_for already resolves to a RouterError subclass — the only _BY_CODE/bucket overlaps are exactly those three, every other bucket hits _BY_ERROR_TYPE, and an unknown bucket falls back to RouterError — so the typed entries are forwarded with no help from this branch. Verified across invalid_input, the three shared buckets and an unknown future bucket: all yield a RouterError with .errors populated.
2. What it still did beyond that was a regression, flagged by 4 of 6 panel reviewers. detail[] is a body shape any server, proxy or gateway can send (a FastAPI RequestValidationError is exactly it), so keying the class off it let an intermediary decide which except a caller runs: a bucket-less 422 stopped raising InvalidWorkflow, the elected RouterError dropped exc.details, and a v2-vocabulary code got stamped onto .error_type. It also contradicted the rule error_from_envelope already documents, learned three times on live traffic — only a response that carries a bucket gets retyped.

The tests that pinned the reroute now pin the opposite: a bucket-less array keeps its v2 class (not isinstance(err, RouterError)), with details intact and the per-field reasons still reaching the caller through the summary.

Scope note / judgment call

The fix chose the "one summariser" option (over merely aligning the low-level format). A side effect is that the queued path's summary is now sanitised and bounded too — the same latent gap the awaited path had. This is deliberate and consistent, and it makes the run/queued .detail parity exact for every body rather than only clean ones. .errors still carries the raw typed entries; only the human-readable summary string is cleaned and field-qualified. This also changes err.message/.detail on the run() path to include the field path (body.steps: too large rather than bare too large) — the pre-existing per-field tests were updated to the corrected value, and there is a CHANGELOG entry for it.

Testing

• ruff check . — clean
• ruff format --check . — clean (57 files)
• mypy src — no issues (21 files)
• pytest — 987 passed, 9 skipped
• Merged origin/main at e4773c7 (which landed fix(errors): make except RouterError catch every Router refusal #157) into the branch; the above was run on the merged tree.

Provenance

• Authored by: agent-work loop
• Verified: on the merged tree (merge commit 9525758): ruff check + ruff format --check + mypy src all clean; pytest 987 passed, 9 skipped. The removal of the to_sdk_error reroute was checked empirically — every Router-provenance bucket still yields a RouterError with .errors populated, and a bucket-less body keeps InvalidWorkflow with its details.
• Deviations: the to_sdk_error reroute described in the original revision is removed, not shipped — subsumed by fix(errors): make except RouterError catch every Router refusal #157 and flagged as a regression by 4 of 6 panel reviewers (see the section above). The sanitise-and-unify core is unchanged. One panel finding is deferred to a follow-up rather than fixed here (below).

Residual

• Deferred to a follow-up: the STRING detail/error.message forms are still passed through with only a .strip() (four sites across comfy_low.errors and router_exceptions), so a server-supplied string with control characters or ANSI escapes still reaches str(exc) unbounded. Out of scope here — this PR is scoped to the array shape, and fixing the string forms changes .detail for every non-validation error on both surfaces. Recorded for filing; the docstring no longer implies the string shape is covered.
• Unexercisable artifact: the original findings came from an automated review panel whose Post review step failed twice and left them only in an expired GitHub Actions run artifact. That artifact is gone and cannot be fetched; it was not needed, as each finding was re-derived and verified against merged main.

[coderabbitai]

Warning

Review paused — included plan limit reached

Keep your review moving with free on-demand reviews.

• Run this review for free

On-demand reviews are free for the next 21 days.

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Promotion and pricing details

On-demand reviews are free for the next 21 days. After that, they cost $0.25 per reviewed file.

Review limit details

Or wait 59 minutes for your next included review.

Check out review usage here.

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: Comfy-Org/comfy-python-sdk/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 37eb3336-a0a5-4b47-9376-a93700b1ae9c

📥 Commits

Reviewing files that changed from the base of the PR and between e4773c7 and 9525758.

📒 Files selected for processing (7)

• CHANGELOG.md
• src/comfy_low/errors.py
• src/comfy_sdk/exceptions.py
• src/comfy_sdk/router_exceptions.py
• tests/test_error_mapping.py
• tests/test_models_run.py
• tests/test_router_exceptions.py

---

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

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 7 finding(s).

Severity	Count
🟠 High	1
🟡 Medium	1
🟢 Low	5

Panel: 5/6 reviewers contributed findings.

Reviewers that did not contribute: gpt-5.6-sol-max:edge-case (error)

[robinjhuang]

Auto-approved under the full-autonomy policy.

Gates verified at 2096320b057367f1cd038c38d3787ec4f707aa44:

• full-autonomy label present
• assigned to, or review requested from, @robinjhuang
• not a draft
• 8 required check(s) green — none failing, none pending

Issued by full-autonomy-approve.yml (run). This approval attests
that the machine gates above passed at this commit. It does not attest that a
human read the diff.

[mattmillerai]

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

• BE-15946 — Sanitise the STRING detail/message error forms, not just the detail[] array — filed as agent-spike (premise unverified)

The following carry agent-spike instead of agent-ok because their reachability claim was not backed by evidence (BE-5378) — the claim is investigated before any code is written, and "the premise does not hold" is a valid, successful outcome:

• Sanitise the STRING detail/message error forms, not just the detail[] array — no reachability block in the proposal

[robinjhuang]

Auto-approved under the full-autonomy policy.

Gates verified at 9525758506a914a54e12416feafe5a8d7c096b3d:

• full-autonomy label present
• assigned to, or review requested from, @robinjhuang
• not a draft
• 8 required check(s) green — none failing, none pending

Issued by full-autonomy-approve.yml (run). This approval attests
that the machine gates above passed at this commit. It does not attest that a
human read the diff.