feat(errors): add Conflict/Timeout/PoolExhausted; structure WS error attrs; fix AuthRequired msg

[TexasCoding]

Wave 0-B of the 2026-05-21 SDK audit umbrella. Errors-hierarchy foundation. Pure additive (new classes + new kwargs on existing classes) — no breaking changes.

Changes

• New exceptions: KalshiConflictError (409), KalshiTimeoutError, KalshiPoolExhaustedError. Re-exported from kalshi (added to __all__). Wave 1-A consumes these in _map_error and the retry loop.
• KalshiSequenceGapError gains kw-only attrs: channel, sid, client_id, last_seq, next_seq.
• KalshiSubscriptionError gains kw-only attrs: channel, client_id, op. Existing (message, error_code=None) positional signature preserved.
• AuthRequiredError default message now mentions both KALSHI_PRIVATE_KEY_PATH and KALSHI_PRIVATE_KEY + links to authentication docs.
• docs/errors.md updated with new classes and structured attrs.

Tests

9 new tests; full test_exports.py + test_client.py suites (233 tests) pass; mypy --strict clean.

Closes #213, Closes #215
Refs #201, Refs #204 (W1-A finishes transport-side wiring)
Refs #224

[claude]

Code Review — PR #226

Wave 0-B: Errors-hierarchy foundation

Overall this is a clean, well-scoped PR. The additive approach (new classes + new kwargs on existing ones) correctly avoids breaking changes, and the structured attrs on KalshiSequenceGapError / KalshiSubscriptionError fill a genuine gap for callers who need context on failures.

---

🔴 Blocker — CI Failing (lint N802)

Four test function names violate ruff's N802 rule (function names must be lowercase). This is why the test matrix is red on both 3.12 and 3.13.

tests/test_client.py:719

# current (fails N802)
def test_default_kwargs_are_None(self) -> None:
# fix
def test_default_kwargs_are_none(self) -> None:

tests/test_exports.py:42, 48, 54

# current (all fail N802)
def test_KalshiConflictError_importable_from_kalshi() -> None:
def test_KalshiTimeoutError_importable_from_kalshi() -> None:
def test_KalshiPoolExhaustedError_importable_from_kalshi() -> None:

# fix (snake_case)
def test_kalshi_conflict_error_importable_from_kalshi() -> None:
def test_kalshi_timeout_error_importable_from_kalshi() -> None:
def test_kalshi_pool_exhausted_error_importable_from_kalshi() -> None:

---

🟡 Minor — Misleading Docstring Wording

KalshiPoolExhaustedError says "Raise KalshiConfig.limits.max_connections" — but "raise" here means the Python keyword, not "increase a config value". This is confusing.

kalshi/errors.py

# current
"""...Raise ``KalshiConfig.limits.max_connections`` if this fires under normal load."""

# clearer
"""...Increase ``KalshiConfig.limits.max_connections`` if this fires under normal load."""

Same wording appears in docs/errors.md:

"you should raise KalshiConfig.limits.max_connections"

---

🟢 What's Good

• Back-compat preserved on KalshiSubscriptionError — error_code stays positional before *; existing callers KalshiSubscriptionError("msg", 1234) still work without changes.
• KalshiSequenceGapError.__init__ — keyword-only attrs with None defaults are the right pattern; no risk of positional arg order bugs.
• Inheritance is correct — KalshiConflictError, KalshiTimeoutError, KalshiPoolExhaustedError all extend KalshiError directly, inheriting the status_code parameter for when Wave 1-A wires _map_error.
• AuthRequiredError message improvement — adding both KALSHI_PRIVATE_KEY_PATH and KALSHI_PRIVATE_KEY and linking to the auth docs is a meaningful UX improvement.
• test_new_exceptions_in_kalshi_all — good catch-all to guard against future __all__ drift.
• test_default_message_mentions_both_env_vars — pins the exact env var names in a test so the message can't silently regress.

---

💭 Suggestions (non-blocking)

Consider defaulting status_code=409 in KalshiConflictError:

class KalshiConflictError(KalshiError):
    """409 Conflict (e.g., duplicate ``client_order_id``)."""

    def __init__(self, message: str, status_code: int | None = 409) -> None:
        super().__init__(message, status_code)

This would let callers who catch KalshiConflictError inspect .status_code without requiring Wave 1-A to pass it explicitly. Minor, and easily deferred to Wave 1-A if you prefer consistency with how _map_error handles all status codes.

Exercise the new exceptions in tests (not just hierarchy checks):

The export tests confirm issubclass relationships but don't instantiate the new classes. A one-liner per class confirms the constructor doesn't blow up unexpectedly:

def test_new_http_exceptions_are_instantiable() -> None:
    assert str(KalshiConflictError("dup order")) == "dup order"
    assert KalshiTimeoutError("timed out").status_code is None
    assert KalshiPoolExhaustedError("pool full").status_code is None

---

Summary: One hard blocker (4 ruff N802 failures) + one minor wording nit. The substantive design is sound. Fix the lint violations and the wording and this is ready to merge.

[claude]

Code Review — PR #226 (Wave 0-B: Errors-hierarchy foundation)

Overall verdict: looks good — a few minor items below.

CI is still pending at time of review. The test names in the current diff are correctly snake_cased, so the N802 issues flagged in the earlier automated comment appear to have been resolved.

---

🟡 Docstring wording — KalshiPoolExhaustedError

errors.py:74

# current
"""...Raise ``KalshiConfig.limits.max_connections`` if this fires under normal load."""
# ↑ "Raise" reads as the Python keyword

# suggested
"""...Increase ``KalshiConfig.limits.max_connections`` if this fires under normal load."""

---

🟡 ASCII tree alignment — docs/errors.md

docs/errors.md:28 — KalshiTimeoutError has an extra leading space before the # comment, breaking the visual column alignment with the other entries:

├── KalshiConflictError              # 409 (e.g., duplicate client_order_id)
├── KalshiTimeoutError                # request timed out ...   ← extra space
├── KalshiPoolExhaustedError         # local pool full ...

---

🟡 client_id attr is untested on both new structured errors

KalshiSequenceGapError.client_id and KalshiSubscriptionError.client_id are never set in tests — only channel, sid, op, last_seq, next_seq are exercised. Happy to accept as-is since the omission is symmetric and the attr is straightforward, but a one-liner assertion would close the gap:

# test_client.py — TestKalshiSequenceGapError.test_carries_channel_sid_seq
assert err.client_id is None  # not set above, verify it defaults

# test_client.py — TestKalshiSubscriptionError.test_carries_channel_op
err2 = KalshiSubscriptionError("sub failed", channel="ticker", op="subscribe", client_id=7)
assert err2.client_id == 7

---

🔵 Hint for Wave 1-A — KalshiConflictError and status_code

KalshiConflictError inherits KalshiError.__init__(message, status_code=None) unchanged. When Wave 1-A wires _map_error, it should pass status_code=409 explicitly — the base class supports it but nothing in this PR enforces it:

# _map_error (Wave 1-A)
if status == 409:
    return KalshiConflictError(message=str(message), status_code=status)

Same pattern used by KalshiAuthError, KalshiNotFoundError, etc. Not a blocker — just flagging so it doesn't get dropped.

---

🔵 URL in AuthRequiredError default message

The hardcoded docs URL (https://texascoding.github.io/kalshi-python-sdk/authentication/) is a nice touch for discoverability, but it will silently rot if the docs ever move. Not worth blocking on, but a comment in the code noting the URL should stay in sync with mkdocs.yml nav could help future maintainers.

---

What's well done

• Purely additive — no breaking changes; error_code positional compat on KalshiSubscriptionError preserved ✓
• Literal type for op is the right call — precise without overhead ✓
• Docstrings on KalshiTimeoutError and KalshiPoolExhaustedError explain the POST idempotency hazard clearly ✓
• Export + __all__ wiring is consistent with the rest of the module ✓
• Test coverage is solid for the main happy/default paths ✓
• Wave scoping is correct — transport wiring deferred to Wave 1-A ✓