fix(models): relax subaccount-number request constraint (drop le=32) (#164)

[TexasCoding]

Closes #164.

TL;DR

The nightly integration job authentication is fine (the workflow ran 224 tests against demo with KALSHI_KEY_ID set). The single failure is a real SDK bug: ApplySubaccountTransferRequest.to_subaccount rejected 41 client-side because the model carried le=32, but the demo server now allocates ephemeral subaccount numbers above 32.

Root cause

The le=32 bound was added in v0.11.0 from a code-review pass that read the spec's prose description (0 for primary, 1-32 for numbered subaccounts) as a hard limit. The actual OpenAPI schema defines no minimum/maximum constraints on any of these fields. Response-side subaccount numbers were already unbounded — the asymmetry between request and response was the latent bug; the nightly test surfaced it the moment demo started allocating above 32.

Failure log:

pydantic_core._pydantic_core.ValidationError: 1 validation error for ApplySubaccountTransferRequest
to_subaccount
  Input should be less than or equal to 32 [type=less_than_equal, input_value=41, input_type=int]

Fix

Dropped le=32 from 7 request-side model fields, kept ge=0:

File	Class	Field
kalshi/models/subaccounts.py	ApplySubaccountTransferRequest	from_subaccount, to_subaccount
kalshi/models/subaccounts.py	UpdateSubaccountNettingRequest	subaccount_number
kalshi/models/orders.py	CreateOrderRequest	subaccount
kalshi/models/orders.py	BatchCancelOrdersV2RequestOrder	subaccount
kalshi/models/communications.py	CreateRFQRequest	subaccount
kalshi/models/communications.py	CreateQuoteRequest	subaccount

Server is the source of truth on the upper bound. Negative values still fail-fast at the SDK boundary (ge=0); out-of-range positives now round-trip cleanly.

Docstrings on ApplySubaccountTransferRequest and SubaccountsResource updated to drop the 1-32 claim and explain the constraint asymmetry so a future review pass doesn't re-introduce the bound from prose alone.

Tests

Replaced test_transfer_request_rejects_out_of_range_subaccount (asserted to_subaccount=33 failed) with test_transfer_request_accepts_subaccount_above_32 (asserts 41 round-trips). The existing test_transfer_request_rejects_negative_subaccount keeps ge=0 coverage.

The new positive-path test is a direct regression guard against the exact failure from issue #164.

Verification

uv run pytest tests/test_subaccounts.py        -> 43 passed in 1.60s
uv run pytest tests/ --ignore=tests/integration -q
                                               -> 2023 passed (unchanged)
uv run mypy kalshi/                            -> Success: no issues
uv run ruff check .                            -> All checks passed!

The failing integration test (test_transfer_between_subaccounts) cannot be exercised locally without KALSHI_KEY_ID, but the new unit-test regression guard directly mirrors the failure mode from the nightly run.

[claude]

Code Review — PR #169: relax subaccount-number le=32 constraint

Verdict: Approve with minor nits. This is a clean, well-reasoned bug fix with clear root-cause analysis. The change is minimal and surgical; the docstring updates are warranted because the spec-prose-vs-JSON-schema asymmetry is exactly the kind of non-obvious invariant worth explaining.

---

What the PR does

Drops the le=32 upper-bound validator from 7 request-side model fields across subaccounts.py, orders.py, and communications.py. The fix is motivated by the demo server allocating ephemeral subaccount numbers above 32 (observed: 41), causing a client-side ValidationError before the request could leave the SDK. The actual OpenAPI schema defines no upper bound; only spec prose mentioned 1–32. The ge=0 lower bound is preserved.

---

Code quality

• Fix is minimal and correct. All 7 fields get the same treatment (ge=0, le=32 → ge=0), no surrounding churn.
• Docstrings are appropriately updated. The "server is the source of truth on the upper bound" language explains the why precisely — this is the kind of non-obvious constraint that deserves a comment (CLAUDE.md §2 agrees).
• CHANGELOG.md entry is clear and thorough — it lists all affected fields and explains the root cause.

---

Potential issues

1. Missing blank line before test_update_netting_request_serializes (minor)

The diff leaves no blank line between the last line of the new test and the next test method:

        assert req.to_subaccount == 41
    def test_update_netting_request_serializes(self) -> None:   # ← missing blank line

This is a style nit but ruff / flake8 E302 would normally flag it. Worth fixing.

2. Only to_subaccount gets a regression test for values > 32

The PR fixes 7 fields but the new test only asserts to_subaccount=41. The following are untested for the above-32 case:

• ApplySubaccountTransferRequest.from_subaccount
• UpdateSubaccountNettingRequest.subaccount_number
• CreateOrderRequest.subaccount
• BatchCancelOrdersV2RequestOrder.subaccount
• CreateRFQRequest.subaccount
• CreateQuoteRequest.subaccount

The fix is identical for all of them so the risk is very low, but a test for at least one more field (e.g., UpdateSubaccountNettingRequest) would widen the regression guard. Up to you whether the marginal confidence is worth the lines.

3. CHANGELOG placement (non-blocking)

The ### Fixed section is appended to the in-progress unreleased block above ## 2.1.0. That looks intentional, but confirm the version bump story is covered — is this targeting a 2.1.1 patch release?

---

Test coverage

• test_transfer_request_accepts_subaccount_above_32 directly mirrors the nightly failure mode — good regression guard.
• test_transfer_request_rejects_negative_subaccount continues to exercise ge=0 — correct.
• Removing test_transfer_request_rejects_out_of_range_subaccount is the right call; keeping a test that asserts wrong behavior is worse than having no test.

---

Security

No concerns. Relaxing a client-side numeric upper bound to match the server spec is not a security change. The server remains the authoritative validator; the SDK is not a security boundary here.

---

Summary

Area	Status
Correctness	✅ Fix matches the spec and the observed failure
Minimal diff	✅ Only the 7 fields, no unrelated changes
Tests	✅ with nit (see item 2 above)
mypy / ruff	✅ per PR description
CHANGELOG	✅
Missing blank line	⚠️ minor formatting issue

Ready to merge after the blank-line nit is resolved.

[TexasCoding]

End-to-end demo verification (local .env has KALSHI_KEY_ID):

On main (pre-fix):

tests/integration/test_subaccounts.py::TestSubaccountsSync::test_transfer_between_subaccounts FAILED
E   pydantic_core._pydantic_core.ValidationError: 1 validation error for ApplySubaccountTransferRequest
E   to_subaccount
E     Input should be less than or equal to 32 [type=less_than_equal, input_value=44, input_type=int]

Reproduces the nightly failure exactly. Demo allocated 44 this time (the nightly run got 41); both > 32 confirms the upper bound was never enforced server-side.

On fix/relax-subaccount-le32-constraint:

tests/integration/test_subaccounts.py::TestSubaccountsSync::test_transfer_between_subaccounts PASSED [100%]
============================== 1 passed in 2.30s ===============================

The fix closes #164 against the live demo API.